File Reading/Writing Problems [Socket Programming]

[MaSSaSLaYeR]

Hello!

First off I'm new here so sorry if my posts arn't so good. Second I'm from Belgian so sorry for my english!

1. Are there any alternatives to fread and fwrite that doesn't require you specifing a buffer size? That directy write output/input
2. Receiving files
   

       FILE *fp;
       fp=fopen("c:\\filereceive.zip", "wb");
   
       int numbytes=0;
       int count =0;
       do {
           numbytes = recv(sockfd, buffer, sizeof(buffer), 0);
   
   
           count = count + numbytes;
   
           if (count >= 33801216 )
           {
               closesocket(sockfd);
               fclose(fp);
           }
           else
           {
               fwrite(buffer, sizeof(buffer), 1, fp);
   
           }
       
       }while(numbytes > 0);

   • I receive the file but I don't know how to close it, the file is always corrupt.
   • I don't know how to send a file transfer complete to the sender; so what happends is that he keeps sending & the connection stays open.

Main Problems:

• Binary files are corrupted when receiving
• Binary files smaller then the buffer size are corrupted
• Can't send file completed over DCC

THank you to any who can help me!
-Ricky-

[CommonTater]

First off you shouldn't be writing sizeof(buffer) you should be writing numbytes after receiving a packet. The packet might be shorter than the buffer, so if you write the whole thing it will get corrupted.

Second... 33,801,216 is going to overflow a 32bit integer so depending on the type of the variable count, you will get errors. count probably needs to be an unsigned long long int (uint64_t) for your code to work.

Third the loop should not exit on numbytes = 0 ... if your sender is working properly every block of the file will be a full buffer long and only the last block will be shorter so you want to exit your loop when numbytes < buffersize.

Finally you don't want a preset size to close the file and you don't want it inside your loop. The signal to close the file should be that the loop has exited.

In pseudocode...

** sender **
unsigned char buffer[1024];
open the file
repeat
  read buffersize bytes into buffer
  send buffer
  if bytes read < buffersize
    exit loop
close file


** receiver **
unsigned char buffer[1024];
open the file
repeat
    bytes = receive up to buffersize bytes into buffer
    write(bytes) of  buffer to disk
    if bytes < buffersize 
      exit loop
close file

[MaSSaSLaYeR]

First off you shouldn't be writing sizeof(buffer) you should be writing numbytes after receiving a packet. The packet might be shorter than the buffer, so if you write the whole thing it will get corrupted.

Third the loop should not exit on numbytes = 0 ... if your sender is working properly every block of the file will be a full buffer long and only the last block will be shorter so you want to exit your loop when numbytes < buffersize.

With buffersize do you mean sizeof(buffer) so 1024? Because when I do it like this:

    do {
        //Receiving From Socket
        numbytes = recv(sockfd, buffer, sizeof(buffer), 0);

        //Counting Bytes Received
        count = count + numbytes;
    /*    if (count >= filesize)
        {
            printf("\n:>File receive Completed\n");
            closesocket(sockfd);
            fclose(fp);
        }
        else
        {*/
            if (bolRecvFile == false)
            {
                printf(":>Receiving file\n");
                bolRecvFile = true;
            }
            fwrite(buffer, numbytes, 1, fp);
        //}

    }while(numbytes < 1024);

This code works:

    do {
        //Receiving From Socket
        numbytes = recv(sockfd, buffer, sizeof(buffer), 0);

        //Counting Bytes Received
        count = count + numbytes;
    /*    if (count >= filesize)
        {
            printf("\n:>File receive Completed\n");
            closesocket(sockfd);
            fclose(fp);
        }
        else
        {*/
            if (bolRecvFile == false)
            {
                printf(":>Receiving file\n");
                bolRecvFile = true;
            }
            fwrite(buffer, numbytes, 1, fp);
        //}

    }while(count < filesize);

    if (count >= filesize)
    {
        printf("\n:>File receive Completed\n");
        closesocket(sockfd);
        fclose(fp);
    }

[CommonTater]

Glad to see it working... but you should not count on sizeof() to return the number of characters in the buffer... it can also return the the size of the pointer to the buffer (usually 4).

#define BUFSIZE 1024

unsigned char buffer[BUFSIZE];

// other stuff

numbytes = recv(sockfd, buffer, BUFFSIZE, 0);

Also, are you explicitly communicating filesize before the transfer begins?
If not, you should be exiting as I said...

do
  {
      receive and write
   }
while (numbytes == BUFFERSIZE);

This format is actually better since it will receive files of unknown size and will terminate the loop on errors.

[Salem]

> while (numbytes == BUFFERSIZE)
recv() can return any number of bytes from 1 to the number you requested, and it still counts as success.

You can't assume that recv() will hang around indefinitely until the buffer is full.

Regardless of how many calls to send() there were, the number of recv() calls which result is completely independent (for a TCP stream). The only guarantee is the ORDER of the bytes. All fragmentation and reassembly is down to the user code.

do {
  n = recv(sockfd, buffer, BUFFSIZE, 0);
  if ( n == 0 ) {
    // remote closed connection
  } else if ( n < 0 ) {
    // an error
  } else {
    // do stuff with 'n' bytes
  }
} while ( n > 0 );

[MK27]

> while (numbytes == BUFFERSIZE)
recv() can return any number of bytes from 1 to the number you requested, and it still counts as success.

[edit: I notice this is obliquely referred to in Salem's code, where "if n == 0"]

Recv() can also return 0, and that sort of counts as success, since -1 is for error.

At least on linux/GNU, this may happen when the other party disconnects. Ie, there is no error, SIGHUP, or SIGPIPE, but the connection will continuously trigger poll() or select() as ready to read, then the read will return 0.

Ie, 0 is a crucial case to deal with. I don't close the connection until it happens a number of times in a row, but maybe that is paranoid since I have not noticed it happen for any other reason but a disconnection.