C++ Win socket sleep bug?

Question

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");
}

Answer 1

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.