0

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.

5
  • 4
    memcpy(&pStruct....hmmmm.... Commented Sep 30, 2016 at 10:44
  • There are 2 errors to begin with. Starting with you haven't allocated anything at all. Commented Sep 30, 2016 at 10:47
  • stackoverflow.com/questions/37549594/… Commented Sep 30, 2016 at 10:53
  • It's not my code, I don't think it works either :-) Commented Sep 30, 2016 at 10:54
  • 1
    You need to find out what source points to. Commented Sep 30, 2016 at 11:12

3 Answers 3

4

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);
}   
Sign up to request clarification or add additional context in comments.

1 Comment

As above..It's not my code, I don't think it works either :-)
0

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));

5 Comments

Why use malloc instead of allocating the struct on the stack?
Don't say "stack" - that's not a language concept. I've now added that possibility.
No, either are valid in C.
@BLUEPIXY: oops. Humble pie. Grammar states sizeof unary-expression or sizeof ( type-name ). You are correct. Yet another difference between C and C++.
As above..It's not my code, I don't think it works either :-)
0

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.

1 Comment

Thanks to all for verifying my thoughts..It's not my code, I don't think it works either :-)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.