Sending char buffer through TCP socket incomplete

Question

I'm just learning how to handle sockets and TCP connections in C. I've got an application (a long one) which basically sends and receives char arrays with the system call write from server to client and vice versa (two separate C applications of course). As long as I use it with a local connection, on the same PC, running the server on a terminal and the client on an another, everything just works fine and the data arrives at the destination. But if I try it with the server on one computer and the client on another but on the same internet line, passing to the client an address like 192.168.1.X (took from the machine on which the server is running), after the connection is established, I've got an error that tells me that the number of expected bytes (which I pass before sending the real char[]) isn't arrived. Same thing if I try the server on my PC, and the client on another one with a different line on a different provider.

There's something I'm missing, are there any limitations in sending a bunch of bytes in sequence?

The code where the error pops up.

SERVER SIDE:

r=htonl(lghstr);
w=write(myFd,&r,sizeof(int));//writes the number of incoming bytes
if(w<0) perror("writeServer4"),exit(-1);
w=write(myFd,tmp->string,lghstr);
if(w<0) perror("writeServer5"),exit(-1);
if(w!=lghstr) perror("ERROR");

CLIENT SIDE

rC=read(fdc,&cod,sizeof(int));//read incoming number of bytes
lghstr=ntohl(cod);
if(rC<0) perror("readClient3"),exit(-1);
rC=read(fdc,dest,lghstr);
if(rC<0) perror("readClient4"),exit(-1);
if(rC!=lghstr) perror("error : "), printf("didn't read the right number of bytes"),exit(-1);

Now this is basically repeated a lot of times, let's even say 300 times, and it's with big numbers that the program doesn't work.

Answer 1

You do ntohl() on one side and not the other. That might be interpreting the bytes with the wrong value.

You should printf() the bytes on both sides and see what the int is being evaluated to.

Edit: I'm convinced this is a programming bug for the record.

If I had to guess, I'd say that you are not synchronous with the other side for some reason. You say this runs 'about 300 times'.

Try adding a magic integer to the protocol.

Heres an example of a client that sends in this order.

1. A magic integer which is always constant.
2. A lengh of bytes about to be sent.
3. The bytes to be sent.

This uses scatter gather mechanics (its nicer for serialization) but other than that it effectively is doing the same thing yours is doing, as a client, just adding a magic value.

When the receiver receives the data, it can validate that the data is coming in the right order, by checking what the magic number was that came in. If the magic is wrong it means the client or server has lost themselves positionally in the stream.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/uio.h>
#include <err.h>

#include <time.h>

#define MAGIC 0xDEADBEEFLU
#define GARBAGE_MAX 65536

const int iterations = 3000;

char * create_garbage_buf(
    void)
{
  int rc = -1;
  int fd = -1;
  char *buf = NULL;

  buf = malloc(GARBAGE_MAX);
  if (!buf)
    err(1, "Cannot allocate buf");

  fd = open("/dev/urandom", O_RDONLY);
  if (fd < 0)
    err(1, "Cannot open urandom");

  rc = read(fd, buf, GARBAGE_MAX);
  if (rc < 0)
    err(1, "Cannot read from urandom");
  else if (rc != GARBAGE_MAX)
    errx(1, "Expected %d bytes, but got %d reading from urandom",
         GARBAGE_MAX, rc);

  close(fd);
  return buf;
}

int main() {
  int fd, offset, i, rc;
  uint32_t magic = MAGIC;
  uint32_t blen = 0;
  char *buf = NULL;
  struct iovec vecs[3];

  /* Seed poor random number generator */
  srand(time(NULL));

  /* Use a file for demonstration, but a socket will do just fine */
  fd = open("/dev/null", O_WRONLY);

  /* Create some garbage to send */
  buf = create_garbage_buf();

  if (fd < 0)
    err(1, "Cannot open file");

  /* The first vector, is always the magic */
  vecs[0].iov_len = sizeof(uint32_t);
  vecs[0].iov_base = &magic;
  for (i=0; i < iterations; i++) {
    /* The second vector represents lengh of what we send
     * in this demonstration it is a number between 0 and
     * GARBAGE_MAX/2.
     */
     blen = rand() % (GARBAGE_MAX / 2);
     vecs[1].iov_len = sizeof(uint32_t);
     vecs[1].iov_base = &blen;
     /* The last record is the data to send. Its another random
      * number between 0 and GARBAGE_MAX which represents the offset
      * in our garbage data to send */
     offset = rand() % (GARBAGE_MAX / 2);
     vecs[2].iov_len = blen;
     vecs[2].iov_base = &buf[offset];

     rc = writev(fd, vecs, 3);
     if (rc < 0)
       err(1, "Could not write data");
     if (rc != (sizeof(uint32_t)*2 + blen))
       errx(1, "Did not write proper number of bytes to handle");

     printf("Wrote %u bytes from offset %u in garbage\n", blen, offset);
  }

  free(buf);
  printf("Done!\n");
  return 0;
}

Answer 2

This is the problem:

rC=read(fdc,dest,lghstr);
...
if(rC!=lghstr) perror("error : ")

The #1 fallacy with socket programming is expecting that recv() and read() will return exactly the same number of bytes corresponding to the write/send call made by the other side.

In reality, partial data is extremely likely and expected. The simple workaround is to loop on read/recv until you get the exact number of bytes expected:

size_t count = 0;
while (count < lghstr)
{
    ssize_t readresult = read(fdc, dest+count, lghstr-count);
    if (readresult == -1)
    {
      // socket error - handle appropriately (typically, just close the connection)
    }
    else if (readresult == 0)
    {
      // The other side closed the connection - handle appropriately (close the connection)
    }
   else
    {
        count += readresult;
    }
}

The other alternative to looping is to the use the MSG_WAITALL flag with the socket. This means, using recv() instead of read(). You'll still need to handle the error cases.

rc = recv(fdc, dest, lghstr, MSG_WAITALL);
if (rc == -1)
{
  // socket error
}
else if (rc == 0)
{
  // socket closed by remote
}
else if (rc < lghstr)
{
   // the other side likely closed the connection and this is residual data (next recv will return 0)
}

Answer 3

Closely read the documentation for read()/write() and learn that those two functions do not necessarily read()/write() as much bytes as they were told to, but few. So looping around such calls counting until all data expected had been read/written is a good idea, not to say an essential necessity.

For examples how this could be done for writing you might like to have look at this answer: https://stackoverflow.com/a/24260280/694576 and for reading on this answer: https://stackoverflow.com/a/20149925/694576