58
votes

I've just come upon yet another code base at work where developers consistently use the address of the first element of structs when copying/comparing/setting, rather than the struct itself. Here's a simple example.

First there's a struct type:

typedef struct {
    int a;
    int b;
} foo_t;

Then there's a function that makes a copy of such a struct:

void bar(foo_t *inp)
{
    foo_t l;
    ...
    memcpy(&l.a, &inp->a, sizeof(foo_t));
    ...
}

I wouldn't myself write a call to memcpy in that way and I started out with suspecting that the original developers simply didn't quite grasp pointers and structs in C. However, now I've seen this in two unrelated code bases, with no common developers so I'm starting to doubt myself.

Why would one want to use this style?

6
None of the answers below actually added anything to my knowledge of C, which sort of was what I was hoping for ;)Magnus
This is pretty useful specifically when multiple types of struct start with the same first element, as a sort of pseudo-inheritance. The CPython codebase uses this heavily with PyObject; all Python objects start with a PyObject member, and they're usually passed around with PyObject * pointers so you don't have to hardcode the actual type of the object you're working with.user2357112 supports Monica
The memcpy becomes safe if this is the final argument: sizeof(foo_t) - offsetof(foo_t, a).Darren Stone
I did stuff like this a fair bit when I was first getting the hang of C structs and arrays. I'd guess your belief as to why it was happening was pretty much spot-on.Fake Name
I would start by not having a single letter variable named l.Guilherme Bernal

6 Answers

62
votes

Instead of that:

memcpy(&l.a, &inp->a, sizeof(foo_t));

you can do that:

memcpy(&l, inp, sizeof(foo_t));

While it can be dangerous and misleading, both statements actually do the same thing here as C guarantees there is no padding before the first structure member.

But the best is just to copy the structure objects using a simple assignment operator:

l = *inp;

Why would one want to use this style?

My guess: ignorance or bad discipline.

61
votes

Nobody should do that. If you rearrange struct members you are in trouble.

16
votes

One wouldn't. If you ever moved a in the struct or you inserted member(s) before it, you would introduce a memory smashing bug.

12
votes

This code is unsafe because rearranging the members of the struct can result in the memcpy accessing beyond the bounds of the struct if member a is no longer the first member.

However, it's conceivable that members are intentionally ordered within the struct and programmer only wants to copy a subset of them, beginning with member a and running until the end of the struct. If that's the case then the code can be made safe with the following change:

    memcpy(&l.a, &inp->a, sizeof(foo_t) - offsetof(foo_t, a));

Now the struct members may be rearranged into any order and this memcpy will never go out of bounds.

1
votes

It's a really bad habit. The struct might have another member prepended, for example. This is an insanely careless habit and I am surprised to read that anyone would do this.

Others have already noted these; the one that bugs me is this:

struct Foo rgFoo [3];
struct Foo *pfoo = &rgFoo [0];

instead of

struct Foo *pfoo = rgfoo;

Why deref the array by index and then take the address again? It's already the address, the only difference of note is that pfoo is technically

struct Foo *const, 

not

struct Foo *.  

Yet I used to see the first one all the time.

1
votes

Actually, there is one legitimate use case for this: constructing a class hierarchy.

When treating structs as a class instances, the first member (i.e. offset 0) will typically be the supertype instance... if a supertype exists. This allows a simple cast to move between using the subtype vs. the supertype. Very useful.

On Darren Stone's note about intention, this is expected when executing OO in the C language.

In any other case, I would suggest avoiding this pattern and accessing the member directly instead, for reasons already cited.