3
votes

I am currently working on a project that uses the network. I have to send a struct

    struct Header
    {
     uint32_t   magic;
     uint32_t   checksum;
     uint32_t   timestamp;
     uint16_t   commandId;
     uint16_t   dataSize;
    };

    struct Packet
    {
     struct Header  header;
     char       data[128];
    };

I'm trying to send the struct Packet from one socket to another using TCP. I've tried to send my struct like that

      send(socket, &my_struct, sizeof(my_struct), 0);

but it is not working so I've tried to serialize my struct into a char*

unsigned char               *Serialization::serialize_uint32(unsigned char *buffer, uint32_t arg)
{
 buffer[3] = (arg >> 24);
 buffer[2] = (arg >> 16);
 buffer[1] = (arg >> 8);
 buffer[0] = (arg);
 return (buffer + sizeof(uint32_t));
}

unsigned char               *Serialization::serialize_uint16(unsigned char *buffer, uint16_t arg)
{
 buffer[1] = (arg >> 8);
 buffer[0] = (arg);
 return (buffer + sizeof(uint16_t));
}

    unsigned char                           *Serialization::deserialize_uint32(unsigned char *buffer, uint32_t *arg)
    {
      memcpy((char*)arg, buffer, sizeof(uint32_t));
      return (buffer + sizeof(uint32_t));
    }

    unsigned char                           *Serialization::deserialize_uint16(unsigned char *buffer, uint16_t *arg)
    {
     memcpy((char*)arg, buffer, sizeof(uint16_t));
     return (buffer + sizeof(uint16_t));
    }

even when a client symply send a struct Header data is corrupt when I read it server side Why is the data corrupt ?

Client sending loop

    TcpSocket                     tcp;
    Packet                        p;
    std::stringstream             ss;
    int                           cpt = 0;
    int                           ret = 0;
    char                          *serialized;

    tcp.connectSocket("127.0.0.1", 4242);
    while (getchar())
    {
      ss.str("");
      ss.clear();
  ss << cpt++;
  p.header.magic = 0;
  p.header.checksum = 1;
  p.header.timestamp = 2;
  p.header.commandId = 3;
  p.header.dataSize = ss.str().length();
  memset(p.data, 0, 128);
  memcpy(p.data, ss.str().c_str(), ss.str().length());
  serialized = new char[sizeof(Header) + ss.str().length()];
  bzero(serialized, sizeof(Header) + ss.str().length());
  Serialization::serialize_packet(serialized, p);
  hexDump("serialized", serialized+1, sizeof(Header) + ss.str().length());
  ret = tcp.write(serialized+1, sizeof(Header) + ss.str().length());
}

server recv loop: (fonction call by select() )

  buff = new char[bav];
  socket->read(buff, bav);
  hexdump("buff", buff, bav);

socket->read() :

    int                     TcpSocket::read(char *buff, int len)
    {
      int                   ret;

      ret = recv(this->_socket, buff, len, 0);
      return (ret);
    }

when I run those programs :

    ./server
    [Server] new connexion :: [5]
    recv returns : 17
    buff serialized:
      0000  00 00 00 00 14 00 00 00 1c 00 00 00 1a 00 00 00  ................
      0010  1b

    ./client
    serialized data:
      0000  00 00 00 00 00 00 01 00 00 00 02 00 03 00 01 30  ...............0
      0010  00
    send returns : 17
3
Are we to assume the client is unpacking this in a proper reverse algorithm (and note you should be using unsigned char for your pack buffer). Also, your structure packing in the first case as well as machine endian format will come into play. - WhozCraig
For uint16 and uint32, you can just use ntohl, ntohs, htonl, and htons. The short answer is this: You must precisely define your data format at the byte level and correctly convert to and from that "wire format" when you send and receive. - David Schwartz
@DavidSchwartz I concur, but when I proffered that up as a solution a couple of days ago for a similar problem I was summarily jumped one-side and down the other because it wasn't in "the standard". Still stinging from that one.(I'd still do it with the POSIX functions regardless, btw, just as you probably would). - WhozCraig
There are two obvious problems. First, send(socket, &my_struct, sizeof(my_struct)) will not work because send() takes four arguments. Second, the serialization should be using unsigned char (i.e. 'byte') instead of char. As has been pointed out, the 'network' functions to obviate endian-ness issues are even better. - arayq2
@WhozCraig: Since this question is tagged C and C++ and uses uint32_t, I can't imagine what "the standard" would be. :) - David Schwartz

3 Answers

5
votes

So, this is wrong, and it will definitely cause errors.

buff = new char[bav];
socket->read(buff, bav);
hexdump("buff", buff, bav);
socket->read() :

int TcpSocket::read(char *buff, int len)
{
    return recv(this->_socket, buff, len, 0);
}

The return value from recv() must not be ignored.

From man 2 recv:

RETURN VALUES
     These calls return the number of bytes received, or -1 if an error
     occurred.

     For TCP sockets, the return value 0 means the peer has closed its half
     side of the connection.

So, how many bytes did you receive? It's impossible to tell, if you discard the result from recv(). Maybe recv() failed, you'd never find out if you don't check the return value. Maybe it only filled up part of your buffer. You have to check the return code from recv(). This is the number one error people make when writing programs that use TCP.

You will need to alter your code to handle the following cases:

  1. The recv() call may completely fill your buffer.

  2. The recv() call may partially fill your buffer.

  3. The recv() call may return 0, indicating that the sender has shut down the connection.

  4. The recv() call may indicate EINTR because it was interrupted by a system call.

  5. The recv() call may indicate ECONNRESET because the sender has closed the connection suddenly or has disappeared.

  6. The recv() call may encounter some other error.

Remember: when using TCP, just because you send() 16 bytes doesn't mean that the other peer will recv() 16 bytes — it may be broken up into chunks. TCP is a stream protocol. Unlike UDP, adjacent chunks of data can be arbitrarily joined or split.

2
votes
  1. You need to mask only the low 8 bits each time:

    buffer[3] = (arg >> 24) & 0xff;
    buffer[2] = (arg >> 16) & 0xff;
    buffer[1] = (arg >> 8) & 0xff;
    buffer[0] = (arg) & 0xff;
    
  2. Do the same when you deserialize

0
votes

If I were you I would not reinvent the wheel. There are a lot of well documented and tested libraries / protocols out there for exactly the purpose you are looking for. A small list which just comes to my mind: