I've got a little problem with sending file with TCP to downloaded server. I spend couple of hours to find what is the problem but still I can't find the reason why it doesn't work.

The main problem is when i'm trying to send a file. Program got number of bytes of my file also read the file and passes through the condition in a loop, but it only sometimes send the file. While i run it by debugger the program always send all the files. Also the problem disappear when i use sleep() for 1 second.

#include <iostream>
#ifndef WIN32_LEAN_AND_MEAN
#define WIN32_LEAN_AND_MEAN
#endif
#include <windows.h>
#include <winsock2.h>
#include <ws2tcpip.h>
#include <iphlpapi.h>
#include <string>
#pragma comment(lib, "ws2_32.lib")
#include <windows.h>
int main()
{
    WSADATA wsaData;
    int iResult = WSAStartup(MAKEWORD(2, 2), &wsaData);
    if (iResult != 0) {
        printf("WSAStartup failed: %d\n", iResult);
        return 1;
    }
    SOCKET SendSocket;
    SendSocket = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
    if (SendSocket < 0) {
        perror("Error at socket: ");
        return 1;
    }

    struct sockaddr_in myAddr;
    memset(&myAddr, '\0', sizeof myAddr);
    myAddr.sin_family = AF_INET; //IP4
    myAddr.sin_port = htons(2000);

    inet_pton(AF_INET, "127.0.0.1", &(myAddr.sin_addr)); 
    memset(myAddr.sin_zero, '\0', sizeof myAddr.sin_zero);
    if (connect(SendSocket, (struct sockaddr*)&myAddr, sizeof(myAddr)) < 0)
    {
        perror("Error at connect: ");
        return 1;
    }

    char buffer[512];

    const char fileName[] = "test.txt";
    FILE* fd;
    if (errno_t a = fopen_s(&fd, fileName, "rb") != 0) {
        perror("File not opened:");
        return 1;
    }

    size_t bytes_read = 0;
    size_t test = 0;
    while (!feof(fd)) {
        do {
            if ((bytes_read = fread(buffer, 1, sizeof(buffer), fd)) < 0)
            {
                std::cout << "while error";
                break;
            }
            else
            {
                //Sleep(1000);
                if ((test = send(SendSocket, buffer, bytes_read, 0)) < 0) {
                    perror("Error send");
                    return 1;
                }

                //std::cout << "while send" << std::endl;
            }
       
        } while (test != bytes_read);
    }
    fclose(fd);
    WSACleanup(); //clinap
    system("PAUSE");
}

CodePudding user response：

Your code is broken. send returns the number of bytes sent, and it can be less than bytes_read. But the loop while (test != bytes_read) throws away the buffer contents and fills it with a new fread.

You could change it to call send in a nested loop until the entire buffer is sent but that will be inefficient with regard to TCP framing - for best performance need to combine fread and send in the same loop.

Also about framing - your buffer is too small (512), the recommended send size is 1500 (the typical max MTU).

Also note: your fread error handling is broken. In Win32 fread return value is unsigned, so it will never be < 0.

It should be something like this (untested):

char buffer[8192];
int bytes_in_buffer = 0;
for (;;) {
    if (bytes_in_buffer < sizeof(buffer) / 2) {
        int bytes_read = (int)fread(buffer   bytes_in_buffer, 1, sizeof(buffer) - bytes_in_buffer, fd);
        if (bytes_read == 0) {
            if (ferror(fd)) {
                perror("fread");
                return 1;
            }
            // at EOF now but continue
        }
        bytes_in_buffer  = bytes_read;
    }
    if (bytes_in_buffer == 0) {
        break; // all data is sent - done
    }
    int bytes_sent = send(SendSocket, buffer, bytes_in_buffer, 0);
    if (bytes_sent < 0) {
        perror("send");
        return 1;
    }
    bytes_in_buffer -= bytes_sent;
    if (bytes_in_buffer > 0 && bytes_sent > 0) {
        memmove(buffer, buffer   bytes_sent, bytes_in_buffer);
    }
}

A possible improvement here is to use a ring buffer so as to reduce copying.

CodePudding user response：

You need to better check the return value of send

https://linux.die.net/man/2/send

Return Value On success, these calls return the number of characters sent.

Sometimes, less than the full buffer will be sent. You can't just discard the rest, you need to re-send them.

You also need to deal with error conditions:

EAGAIN or EWOULDBLOCK The socket is marked nonblocking and the requested operation would block. POSIX.1-2001 allows either error to be returned for this case, and does not require these constants to have the same value, so a portable application should check for both possibilities.

Those are the most likely to be affected by Sleep. If you sleep a lot, you won't be getting those errors because the message gets to be flushed while you slept. When you don't sleep, getting those mean you need to wait a bit (Sleep(0)) then try again until you get a positive return value. Don't just error out because you got them once.