Packet parsing in C using struct

Question

I have a packet coming in from the wire and I have a struct that represents the packet:

typedef struct {
    uint8_t packetType;
    uint8_t remainingLength;
    uint16_t protocolNameLength;
    char protocolName[20];
    uint8_t protocolLevel;
    uint8_t connectFlags;
    uint16_t keepAlive;
} Connect;

I have the following C code that parses the packet:

void decodePacket(char *packet) {
 Connect *connect = NULL;
 connect = (Connect *) packet;
 // Here I access the different fields of the packet using the connect struct
 printf("Protocol Name is %s\n", connect->protocolName);
}

In the above protocol the strings are not null ternimated when they come in from the wire and hence I get some wierd characters when I print the protocol name. Is there any way to solve this without changing the protocol?

Also if the protocol name is > 20, the char buffer will overflow. Is there a way to solve this problem? Do I need to drop this approach of parsing packets and just use an index and parse each byte of the packet manually?

Thanks

Answer 1

The cast of the packet to the struct is incredibly dangerous, as you have little or no control of how the struct will actually be aligned in memory.

You need to read The Lost Art of C Structure Packing: http://www.catb.org/esr/structure-packing/

Answer 2

There are a few ways; which is best depends on many factors.

1st way. If you read your packet a byte at a time, using a pointer or an index to write in the receiving buffer, then you know how many chars will come in, for protocolName, after having read protocolNameLength. Then you can 1) discard trailing chars in names too long for your struct; 2) add a NULL at the end of protocolName, so you can easily manage the name; 3) move the writing pointer in the correct location after having read protocolName. You can also reserve as many chars you want for protocolName, by declaring it as "char protocolName[many]": it will not have a big cost.

2nd way. You can receive a packet in a burst, in a buffer, and then memcpy() the received buffer into your struct, in two different operations. The first memcpy copies the first 4 members; the second copies the rest of the packet in the correct destination. Some pointer arithmetic has to be done for the second memcpy(). The first memcpy() would protect from names too long. Again, you can add a NULL at the end of the data copied with the first memcpy, so you have a real C string for protocolName.

3rd way. You can use, as receiving buffer, the struct itself, and then memmove() from the "real (received)" position of protocolLevel to the "correct (designed)" address. This memmove would always move data backward, assuming that you reserved enough space for protocolName[]. Again, you can put a NULL in the correct place if protocolName (or in the last element, 19 (20-1) in your example).

Given that protocolNameLength is declared with 16 bits, I can suppose that names could be very long, and it is strange. But if really this name can be so lengthy, then moving memory around would waste CPU.

Playing around with memcpy() / memmove() would have sense if you have many fields to manage, and you have to do that many times. And it is viable if you don't have big blocks to move, and you know how the compiler packs/pads the struct (this last requirement is needed anyway, if you want to superimpose a C struct on a wire protocol).

Personally I would prefer the first way, if possible; otherwise, if fields are not too many, parse the packet "by hand".

Answer 3

In the above protocol the strings are not null terminated when they come in ... I get some weird characters when I print the protocol name. Is there any way to solve this without changing the protocol?

Limit the printing width with a precision for %s

If a precision is specified, no more than that many bytes are written C11dr §7.21.6.1 8

// printf("Protocol Name is %s\n", connect->protocolName);
printf("Protocol Name is %.*s\n", 
    (int)(sizeof connect->protocolName), 
    connect->protocolName);

"the strings are not null terminated" is a contradiction. In C, a string, as used in the standard library, is always null character terminated, else it is not a string, but simply a character array. Fortunately %.*s handles character arrays and strings, stopping at the given precision or the null character.

---

The following code is a problem @M.M with char * including alignment of Connect *. This can be solved with the higher un-posted code or as below. Using char* to decode packets is not a roust solution.

void decodePacket(char *packet) {
   Connect *connect = NULL;
   connect = (Connect *) packet;  // bad

// alternative
void decodePacket(char *packet) {
   Connect connect;
   memcpy(&connect, packet, sizeof connect);
   printf("Protocol Name is %.*s\n", 
       (int)(sizeof connect.protocolName), 
       connect.protocolName);

---

Also if the protocol name is > 20, the char buffer will overflow.

This is unclear. If the overflow occurs in the sender, then the receiving code can not fix that. OP's printf() code can be fixed as described above. OP has no other code that overflows any buffer.