2

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.

Nick S
  • 1,299
  • 1
  • 11
  • 23
  • 1
    Here `buffer = malloc(size); while (((len = recv(fd, buffer, sizeof(buffer), ...` the operator `sizeof` does not do what you expect. It returns the size of a pointer (4 or 8). It *does not* return `size`. – alk Aug 13 '17 at 11:46
  • Also you want the client to abort operation immediately when `rev()` returned `0` as well. As this tells the client, that the server shut down the connection. The client would not be able to receive anything any more via the current socket descriptor. – alk Aug 13 '17 at 11:59
  • 1
    To transfer the file's name the code sends `strlen(rx_tx_file->in_name)+1` bytes and receives `sizeof(rx_tx_file->out_name)` bytes, which most likely is different, then the number sent. – alk Aug 13 '17 at 12:29
  • I update questions, about how it should look to recv data from server in a while, its seems if i start to recv data in loop on client side, happens something like missmatch, in server one loop faster than another, on client side i start recv name of file and md5 sum in the same loop, when i pass to the next one all date was recv in prev loop and its stuck. – Nick S Aug 14 '17 at 07:39
  • On how to receive/read n bytes via a TCP socket have look at this answer I gave on a similar issue: https://stackoverflow.com/a/20149925/694576 – alk Aug 14 '17 at 07:48
  • the receive/send loops only 'seem' to not be working because statements like: `printf("remain %d", remain_data);` are missing the '\n' at the end of the format string, so the output is sitting in the stdout buffer rather than being immediately displayed on the terminal. – user3629249 Aug 14 '17 at 16:48

2 Answers2

5

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.

Andrew Henle
  • 32,625
  • 3
  • 24
  • 56
  • "*Since the first two strings don't have a fixed number of bytes ...*" the MD5 checksum most likely is the same size always: 32 bytes – alk Aug 13 '17 at 11:51
  • Filename (file_in) and md5sum its a char file_in[128], md5sum[128], defined in my struct tx_rx_file.I do everything in order on the client side, send file_name, recv file_name and so on... Your advice is to use loops? when send and revc every time? Its would be not srange to have 3 while on client side and 4-5 while on server side? – Nick S Aug 13 '17 at 12:17
  • @NickS no. That is normal behaviour with octet, (byte), streams. – Martin James Aug 13 '17 at 12:52
2

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

Pras
  • 4,047
  • 10
  • 20
  • But I don't know which size I should receive (file size for example) how I should write condition for it? – Nick S Aug 13 '17 at 14:21
  • 1
    @NickS: You won't get around to define a protocol. All TCP-based applications use one or the other. – alk Aug 13 '17 at 16:07