Serialize struct to byte array with pointer as struct member

Question

I am trying to copy a byte array to my struct, then serialize my struct to a byte array again.

But, after I serialize my struct array, I cant get my data value (0x12, 0x34, 0x56) again, instead i get some rubbish data.

What is wrong here?

#pragma pack(push, 1)
typedef struct {
    uint8_t length;
    uint8_t *data;
} Tx_Packet;
#pragma pack(pop)

static void create_tx_packet(uint8_t *packet, uint8_t *src, int length);

int main(void)
{
    uint8_t packet[32];
    uint8_t data[] = { 0x12, 0x34, 0x56 };

    create_tx_packet(packet, data, 3);

    //i check using debugger, i cant get the data value correctly
    //but i could get length value correctly


    return 0;
}

static void create_tx_packet(uint8_t *packet, uint8_t *src, int length)
{
    Tx_Packet *tx_packet = malloc(sizeof(*tx_packet ));

    tx_packet->length = length;
    tx_packet->data = (uint8_t *)malloc(length);
    memcpy(tx_packet->data, src, length);

    memcpy(packet, tx_packet, sizeof(*tx_packet));
}

to compile, the following headers are needed: #include <stdio.h> #include <stdlib.h> #include <string.h> #include <stdint.h> — user3629249
– user3629249, Commented Jun 18, 2015 at 21:33

Dmitri · Accepted Answer · 2015-06-17 05:56:10Z

3

Right now, your create_tx_packet() function copies a Tx_Packet struct created in the function to a uint8_t array. That struct contains the length and a pointer to the data, but not the data itself. It's actually not necessary to use the struct as an intermediate step at all, particularly for such a simple packet, so you could instead do:

static void create_tx_packet(uint8_t *packet, uint8_t *src, int length)
{
  *packet = length; /* set (first) uint8_t pointed to by packet to the
                       length */
  memcpy(packet + 1, src, length);  /* copy length bytes from src to
                                       the 2nd and subsequent bytes of
                                       packet */
}

You still need to make sure packet points to enough space (at least length + 1 bytes) for everything (which it does). Since the version above doesn't dynamically allocate anything, it also fixes the memory leaks in your original (which should have freed tx_packet->data and tx_packet before exiting).

--

If you do want to use a struct, you can (since the data is at the end) change your struct to use an array instead of a pointer for data -- then extra space past the size of the struct can be used for the data, and accessed through the data array in the struct. The struct might be:

typedef struct {
  uint8_t length;
  uint8_t data[];
} Tx_Packet;

and the function becomes (if a temporary struct is used):

static void create_tx_packet(uint8_t *packet, uint8_t *src, int length)
{
  /* allocate the temporary struct, with extra space at the end for the
     data */
  Tx_Packet *tx_packet = malloc(sizeof(Tx_Packet)+length);

  /* fill the struct (set length, copy data from src) */
  tx_packet->length = length;
  memcpy(tx_packet->data, src, length);

  /* copy the struct and following data to the output array */
  memcpy(packet, tx_packet, sizeof(Tx_Packet) + length);

  /* and remember to free our temporary struct/data */
  free(tx_packet);
}

Rather than allocate a temporary struct, though, you could also use struct pointer to access the byte array in packet directly and avoid the extra memory allocation:

static void create_tx_packet(uint8_t *packet, uint8_t *src, int length)
{
  /* Set a Tx_Packet pointer to point at the output array */
  Tx_Packet *tx_packet = (Tx_Packet *)packet;

  /* Fill out the struct as before, but this time directly into the
     output array so we don't need to allocate and copy so much */
  tx_packet->length = length;
  memcpy(tx_packet->data, src, length);
}

edited Jun 17, 2015 at 5:56

answered Jun 17, 2015 at 2:18

Dmitri

9,4052 gold badges30 silver badges36 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Tim Over a year ago

that is a good solution! thank you! :), but that is just a simplify version, but if i do want to fix that problem, what should i do?

Dmitri Over a year ago

You could change your struct to use an array at the end instead of a pointer, and allocate extra space for the data when you allocate the struct.

Tim Over a year ago

yup that what i thought, but then i wanted to use pointer cuz is more efficient (dynamically allocate the memory) but i found this problem, so if i would like to stick using pointer the solution would be?

Carey Gregory Over a year ago

@Tim Allocate the entire struct dynamically and add the size of your array to the allocation.

Tim Over a year ago

thank you! i love the last solution, is very neat and clean! :)

dvhh · Accepted Answer · 2015-06-17 02:12:29Z

2

If you use memcpy(packet, tx_packet, sizeof(*tx_packet)); you are copying the memory representation of tx_Packet into packet, starting with tx_packet->length.

Additionally when mallocating tx_packet that size should be sizeof(*packet)+sizeof(uint8_t) (length of packet plus length field)

And again when copying the tx_packet back to packet you are writing out of the boundaries of packet.

EDIT:
I forgot to mention that depending on your compiler memory alignment parameter you could get any length for the fields (including tx_packet->length) to accelerate memory operation. On 32bits machine it could be 4 and padded with rubbish.

edited Jun 17, 2015 at 2:12

answered Jun 17, 2015 at 1:47

dvhh

4,73829 silver badges34 bronze badges

1 Comment

Tim Over a year ago

thanks for the info, i am using 32 bits, and i have set pragma to remove the padding. but i still dont know how can i store my data in my struct

Steve Summit · Accepted Answer · 2015-06-17 01:47:34Z

0

When you serialize your struct with

memcpy(packet, tx_packet, sizeof(*tx_packet));

you're copying the length and the pointer to the data, but not the data itself. You'll probably need two memcpy calls: one of sizeof(uint8_t) to copy the length field, and one of length to copy the data.

answered Jun 17, 2015 at 1:47

Steve Summit

49.2k9 gold badges80 silver badges113 bronze badges

Comments

Chris Dodd · Accepted Answer · 2015-06-17 02:26:38Z

0

This line:

Tx_Packet *tx_packet = malloc(sizeof(*packet));

only allocates one byte for the packet header, which you then immediately write off the end of, causing undefined behavior. You probably meant

Tx_Packet *tx_packet = malloc(sizeof(*tx_packet));

answered Jun 17, 2015 at 2:26

Chris Dodd

127k14 gold badges150 silver badges243 bronze badges

1 Comment

Tim Over a year ago

sorry it is a typo there, i have change it

Collectives™ on Stack Overflow

Serialize struct to byte array with pointer as struct member

4 Answers 4

5 Comments

1 Comment

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

5 Comments

1 Comment

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related