Stuck in while when transfer file through socket using TCP

Question

I write program and it works fine, but i want to rewrite it using sendfile() and now i got stuck in a loop.

Server side:

• send name = ok
• send md5 checksum = ok
• send size = ok
• send file = ko

Client side:

• recv name = ok
• recv md5 cecksum = ok
• recv size = ok
• create dir and create file = ok
• write data to created file = ko

P.S In previous version of program i stuck some time to, but it depend how much i use printf why? for e.x i add one line with printf program stuck, delete it, works fine.

UPDT: rewrite code client/server

client

/* Received file name */
  int rc_byte = 0;
  rc_byte = recv(fd, rx_tx_file->out_name, sizeof(rx_tx_file->out_name),0);
  if (rc_byte < 0){
    perror("Failed to receive file name: ");
    exit(-1);
  } else
    printf("Recv out name %s\n", rx_tx_file->out_name);
  //printf("file name rc %s\n", rx_tx_file->out_name);                                                                           
  trimm_path_name(rx_tx_file);


  /* Received md5sum */
  rc_byte = recv(fd, rx_tx_file->md5sum, sizeof(rx_tx_file->md5sum), 0);
  if (rc_byte < 0) {
    perror("Failed to receive check sum: ");
    exit(-1);
  } else
    printf("recv md5s %s\n", rx_tx_file->md5sum);


  /* Received file size */
  rc_byte = recv(fd, &size, sizeof(size), 0);
  if(rc_byte < 0) {
    perror("Recevid size of file: ");
     exit(-1);
  }
  printf("%d recv size\n", size);
  to_read = size;

  if (stat(dir, &st) == -1){
    mkdir(dir, 0777);
  }

send_data: (add func to server)

void send_data(int client_fd, m_file *rx_tx_file, int option, int size) {

  int send_byte = 0;
  int total_send = 0;
  if (option == SEND_NAME) {
    while (total_send < strlen(rx_tx_file->in_name)) {
      send_byte = send(client_fd, rx_tx_file->in_name, sizeof(rx_tx_file->in_name),0);
      if(send_byte == -1) {
        perror("Failed to send file name to client: ");
        exit(SEND_TO_CLIENT_ERROR);
      }
      total_send += send_byte;
    }
  }
  else if (option == SEND_MD5) {
    total_send = 0;
    send_byte = 0;

    while (total_send < strlen(rx_tx_file->md5sum)) {
      send_byte = send(client_fd, rx_tx_file->md5sum, sizeof(rx_tx_file->md5sum),0);
      if(send_byte == -1){
        perror("Failed to send file md5sum to client: ");
        exit(-1);
      }
      total_send += send_byte;
    }
  }
  else if (option == SEND_SIZE) {
    send_byte = send(client_fd, &size, sizeof(size),0);
    if (send_byte == -1) {
      perror("Failed to send size: ");
    }
  }
}

server:

client_fd = accept(server_fd, (struct sockaddr*) &client_addr, &length)
    /*send name of file*/
    send_data(client_fd, rx_tx_file, SEND_NAME, 0);
    /*send md5 sum*/
    take_check_sum(rx_tx_file,rx_tx_file->file_in, 0);
    send_data(client_fd, rx_tx_file, SEND_MD5, 0);
    /*send size of file*/
    size = stats.st_size;
    send_data(client_fd, rx_tx_file, SEND_SIZE, size);

    remain_data = stats.st_size;
    printf("File [%s] ready to send\ncheck sum [%s]\n", rx_tx_file->in_name,rx_tx_file->md5sum);
    while (((send_byte = sendfile(client_fd, file_fd, &offset, size)) > 0) && (remain_data > 0))
      {
        remain_data -= send_byte;
        printf("remain %d", remain_data);
      }
    printf("Succesfully");

Since i work with one client and pass file which should send on server side through command line args, i dont need to wait in while (client_fd = accpet) i just work with one connection and close server. Now its work good. But one question is open, how i should rewrite client side to recv data in a loop. I don't know which size i should recv and because of that i cant write right condition to my while loop. THX all for helping.

Answer 1

You should avoid using strlen here in your server:

if(send(client_fd, rx_tx_file->in_name, strlen(rx_tx_file->in_name)+1,0) == -1)

Rather just send fixed length string of size sizeof(rx_tx_file->out_name) as you expect in your client If the filename is smaller just pad it with spaces to make it of length sizeof(rx_tx_file->out_name)

You should also put each receive call in while loop, and add checks that it actually received expected number of bytes, at times recv will just return partial data, you need to post another recv to receive rest of the expected data

Answer 2

TCP is a stream. It has no message boundaries. Your code won't work because of that.

First, you send the name of the file:

send(client_fd, rx_tx_file->in_name, strlen(rx_tx_file->in_name)+1,0)

then you immediately send the md5 sum and then the file size:

send(client_fd, rx_tx_file->md5sum, strlen(rx_tx_file->md5sum)+1, 0)

send(client_fd, &size, sizeof(int),0)

Since the first two strings don't have a fixed number of bytes, it's quite likely that when you try to read the file size or md5 sum from the server you also read the size of the file and maybe even some of the file data.

First, stop trying to put as much of your send and read code as you can into the conditional clause of your if and while statements.

What exactly does

if (send(client_fd, rx_tx_file->md5sum, strlen(rx_tx_file->md5sum)+1, 0) == -1) {
  perror("Failed to send file md5sum to client: ");
  exit(-1);
}

gain you over

ssize_t bytes_sent = send(client_fd, rx_tx_file->md5sum, strlen(rx_tx_file->md5sum)+1, 0);
if ( bytes_sent < 0 )
{
  perror("Failed to send file md5sum to client: ");
  exit(-1);
}

Putting all that code into the if clause gains you nothing on the send. And what if strlen(rx_tx_file->md5sum)+1 is 87 and the send() call returns 15? That's a possible return value that your code can't handle because it stuffs everything into the if clause.

ssize_t bytes_sent = send(client_fd, rx_tx_file->md5sum, strlen(rx_tx_file->md5sum)+1, 0);
if ( bytes_sent < 0 )
{
  perror("Failed to send file md5sum to client: ");
  exit(-1);
}
else if ( bytes_sent < strlen(rx_tx_file->md5sum)+1 )
{
    // partial send...
}

That's actually better coded as a loop.

You didn't post your receive code, but if it's in the same style you not only don't gain anything, by putting everything into the if clause you again can't do any decent error detection or correction.

If your file name recv code is similar to

char filename[1024];
if (recv(fd, &filename, sizeof(filename), 0) < 0) {
   perror("Failed to read file name: ");
   exit(-1);
}

you can't tell what you just received. How many bytes did you just receive? You may have received the file name. You may have received only part of the file name. You may have received the file name, the md5 sum, and some of the file contents itself.

You don't know what you received, and with your code you can't tell. If you zero out the file name and md5 receive buffers and only recv up to one byte less than the size of the buffer, you at least avoid undefined behavior. But if you don't zero out the buffer, or if you read up the the last byte of the buffer, you can also wind up without a nul-terminated string for your filename or md5 sum. And when you then try to treat it as a nul-terminated string you get undefined behavior.

And if you did get extra bytes in the recv calls you make before trying to read the file data, that explains why your code gets stuck - it already read some of the file contents before getting to the loop, so the loop will never see all the content - some of it is gone.