Sending and receiving files in C win32 using a socket

Question

I have a piece of code to send and receive files on Windows with C. Is this the right way to do it? And am I guaranteed that the full file will be sent and received?

Receiving function:

int recvFile(char path[100], SOCKET y)
{
 char buf[BUFSIZE];
 size_t size = BUFSIZE;
 int counter = 0;
 int totalRead = 0;
 // receiving file size
 char b[sizeof(size_t)];
 size_t intTotalReceived = 0;
 size_t intReceived = 0;
 int intSize = sizeof(size_t);
 while (intTotalReceived < intSize)
 {
 intReceived = recv(y, b + intTotalReceived, intSize - intTotalReceived, 0);
 if (intReceived == SOCKET_ERROR)
 {
 printf("error recv buffer size\n");
 }
 intTotalReceived += intReceived;
 }
 size_t test = ntohl_ch(&b[0]);
 if ((int)test == 666666)
 {
 return 1;
 }
 FILE* copyFile;
 fopen_s(&copyFile, path, "wb");
 if (copyFile == NULL)
 {
 printf("Error opening file\n");
 return 0;
 }
 while (totalRead < (int)test)
 {
 int res = recv(y, buf, BUFSIZE, 0);
 if (res == SOCKET_ERROR)
 {
 printf("3 error %d\n", WSAGetLastError());
 return 0;
 }
 size = fwrite(buf, 1, res, copyFile);
 counter++;
 totalRead += res;
 }
 fclose(copyFile);
 return 1;
}

Sending function:

int sendFile(char path[100])
{
 FILE* originalFile;
 errno_t fr = fopen_s(&originalFile, path, "rb");
 if (fr != 0)
 {
 printf("Error opening file\n");
 int n = 666666;
 size_t numb = htonl(n);
 char* converted_num = (char*)&numb;
 size_t intSize = sizeof(size_t);
 size_t intSent = 0;
 size_t intTotalSent = 0;
 while (intTotalSent < intSize)
 {
 intSent = send(ClientSocket, converted_num + intTotalSent, intSize - intTotalSent, 0);
 if (intSent == SOCKET_ERROR)
 {
 printf("error send %d\n", WSAGetLastError());
 break;
 }
 intTotalSent += intSent;
 }
 return 0;
 }
 char buf[BUFSIZE];
 size_t size = BUFSIZE;
 int counter = 0;
 // getting file size
 int sizeFile = getSizeFile(originalFile);
 // sending file size
 size_t num = htonl(sizeFile);
 char* converted_num = (char*)&num;
 size_t intSize = sizeof(size_t);
 size_t intSent = 0;
 size_t intTotalSent = 0;
 while (intTotalSent < intSize)
 {
 intSent = send(ClientSocket, converted_num + intTotalSent, intSize - intTotalSent, 0);
 if (intSent == SOCKET_ERROR)
 {
 printf("error send %d\n", WSAGetLastError());
 return 0;
 }
 intTotalSent += intSent;
 }
 while (size == BUFSIZE)
 {
 size = fread(buf, 1, BUFSIZE, originalFile);
 int r = send(ClientSocket, buf, size, 0);
 if (r == SOCKET_ERROR)
 {
 printf("1 error: %d\n", WSAGetLastError());
 break;
 }
 counter++;
 }
 fclose(originalFile);
 return 1;
}

And then in main (receiving part):

int r = recvFile(path, y);
if (r == 0)
{
 printf("file error\n");
}

Main (sending part):

int ret = sendFile(path);
if (ret == 0)
{
 printf("error sending file\n");
}

• chux: "And am I guaranteed that the full file will be sent and received?" --> No guarantee. Good code looks for potential problems in communications.
• Y K: Yes, I'm doing that using the while loop. If the full buffer doesn't get sent, I sent the rest.

Answer 1

Various size_t

As the size_t is not specified to be the same across Windows, nor the compile version of the sender and receiver code the same, nor is the size of file limited to SIZE_MAX, I would consider instead a fixed size like uint64_t and have the receiver test for fit if needed.

Good that even though OP has "windows", sending/receiving the size is done in network byte-endian order.

Lack of error handling

On error, code should break the loop or return.

 if (intReceived == SOCKET_ERROR)
 {
 printf("error recv buffer size\n");
 // Add return, do not go on.
 }
 intTotalReceived += intReceived;

size = fwrite(buf, 1, res, copyFile); does not test the return value for being less than expected.

Minor: Use const

As int sendFile(char path[100]) does not change path[], consider const.

// int sendFile(char path[100])
int sendFile(const char path[100])

The naked magic number 100 deserves description. As I see it there is no need to limit to 100.

int sendFile(const char *path)

Why send 0 bytes?

If the return value of fread() is 0, little reason to send.

size = fread(buf, 1, BUFSIZE, originalFile);
// Perhaps add `if (size == 0) break;` or change outer loop stop condition.
int r = send(ClientSocket, buf, size, 0);

Files bigger than INT_MAX

Code will certainly not meet "And am I guaranteed that the full file will be sent and received" in this case.

Lots of int variables that mess up send large files like totalRead < (int)test. I recommend uint64_t.

int sizeFile = getSizeFile(originalFile); is also such a problem.

Clean-up on errors

Are all resources clean-up on error?

Example: printf("3 error %d\n", WSAGetLastError()); return 0; should also fclose() the file.

Opinion: 0 implies error?

More C idiomatic to have 0 imply success. IOWs, return an error code.

// if (r == 0) {
if (r) {
 printf("file error %d\n", r);

To the next level

Communication is wrought with lots of pitfalls. Consider forming a 64-bit check code on the data sent and append this to the end. The receiver can use this to verify success.

Consider streaming

Rather than send a file length, simple send a packet of file data with the first bytes indicating packet size. This helps with files whose length is indeterminate at the beginning like sending stdin.

Errors on stderr

Error message deserve to be sent on stderr, not stdout.

For wider application, I would forego any message and return error codes and let the caller print anything if desired.