C: Allocating structs using memcpy

Question

I am looking at some code somewhat as below:

void whatDoesThisDo(uint8_t *source)
{
    STRUCT_T *pStruct;
    memcpy(&pStruct, source, sizeof(STRUCT_T));

    // this function does some stuff with struct contents
    useStruct(pStruct);
}

The intention of the function is to populate the struct from buffer 'source' and then call 'useStruct' (which updates a global based on the contents of the pointer to struct passed to it).

I think the code allocates memory for the pointer on the stack (so pointing to some random location), memcopy then pushes bytes from 'source' (overwriting pStruct, so that now points somewhere else), and useStruct uses content of pStruct as pointer to struct.

Answer 1

The original code:

STRUCT_T *pStruct;
memcpy(&pStruct, source, sizeof(STRUCT_T));

Copies STRUCT_T into STRUCT_T*, which is an programming error.

What you probably want is:

void whatDoesThisDo(uint8_t *source)
{
    STRUCT_T Struct; // Allocate a Struct on the stack.
    memcpy(&Struct, source, sizeof(STRUCT_T));

    // this function does some stuff with struct contents
    useStruct(&Struct);
}

That assumes that source points to a STRUCT_T.

If source is correctly aligned for STRUCT_T, then this function could be just:

void whatDoesThisDo(uint8_t *source)
{
    useStruct((STRUCT_T*)source);
}   

Answer 2

This will not currently work.

If you want pStruct to have dynamic storage duration, then you need to allocate memory for the destination buffer pStruct yourself. Use malloc for that. Then use memcpy(pStruct, source, sizeof(STRUCT_T)); to copy the structure. Note that I pass pStruct, not an address to memcpy.

Alternatively, you could define pStruct to have automatic storage duration; i.e. write

STRUCT_T pStruct;

(perhaps renaming the variable so it doesn't look like a pointer type), and use memcpy(&pStruct, source, sizeof(STRUCT_T));

Answer 3

The intention of the function is to populate the struct from buffer 'source'

You haven't created any struct to populate.

and then call 'useStruct' (which updates a global based on the contents of the pointer to struct passed to it).

That part is OK.

I think the code allocates memory for the pointer on the stack (so pointing to some random location),

Yes. memory for the pointer is the key here. Not memory for the struct.

memcopy then pushes bytes from 'source' (overwriting pStruct, so that now points somewhere else),

Yes. But you overwrote the pointer with the struct itself. The pointer doesn't make any sense now. Plus, the pointer is typically much smaller than a struct, so you overwrote not just the pointer, but also some memory after it. Which is very bad thing, because nobody can tell who used that memory and for what, so there possibly was something important there and you've destabilized your program now.

and useStruct uses content of pStruct as pointer to struct.

Yes, it uses content of pStruct as pointer to struct - but as I said it now contains the struct itself, not just a pointer.

You must treat the pointer as a distinct type than what it points at. Eg my finger is a pointer and I am pointing it at a Statue Of Liberty. What you did is you copied the content of Statue Of Liberty into a finger. It won't fit. What you need is to get another Statue-sized memory place, copy the original Statue there and then you can take a finger and point it into new Statue.