views:

704

answers:

9

During a code review I've come across some code that defines a simple structure as follows:

class foo {
   unsigned char a;
   unsigned char b;
   unsigned char c;
}

Elsewhere, an array of these objects is defined:

foo listOfFoos[SOME_NUM];

Later, the structures are raw-copied into a buffer:

memcpy(pBuff,listOfFoos,3*SOME_NUM);

This code relies on the assumptions that: a.) The size of foo is 3, and no padding is applied, and b.) An array of these objects is packed with no padding between them.

I've tried it with GNU on two platforms (RedHat 64b, Solaris 9), and it worked on both.

Are the assumptions above valid? If not, under what conditions (e.g. change in OS/compiler) might they fail?

+16  A: 

it would definitely be safer to do sizeof(foo) * SOME_NUM

ThePosey
not just safer, but clearer and gets rid of a magic number. +1
rmeador
Yes, I agree with that. I guess I was more trying to get at the padding and array organization. Thanks.
Adam
this does not account for padding between array elements though.
nschmidt
see my answer below. the safest way is to use sizeof(listOfFoos)
nschmidt
@nschmidt: padding between array elements is not allowed in either C or C++.
Jerry Coffin
@Jerry Coffin: you are right. The standard requires arrays to be contiguous. Padding occurs only within the struct/class.
nschmidt
+2  A: 

G'day,

I would've been safe and replaced the magic number 3 with a sizeof(foo) I reckon.

My guess is that code optimised for future processor architectures will probably introduce some form of padding.

And trying to track down that sort of bug is a right pain!

HTH

cheers,

Rob Wells
+12  A: 

An array of objects is required to be contiguous, so there's never padding between the objects, though padding can be added to the end of an object (producing nearly the same effect).

Given that you're working with char's, the assumptions are probably right more often than not, but the C++ standard certainly doesn't guarantee it. A different compiler, or even just a change in the flags passed to your current compiler could result in padding being inserted between the elements of the struct or following the last element of the struct, or both.

Jerry Coffin
It certainly wouldn't surprise me if a compiler decided it liked things on four-byte boundaries, and put a byte of padding at the end.
David Thornley
Unfortunately most don't.
Crashworks
A: 

As others have said, using sizeof(foo) is a safer bet. Some compilers (especially esoteric ones in the embedded world) will add a 4-byte header to classes. Others can do funky memory-alignment tricks, depending upon your compiler settings.

For a mainstream platform, you're probably alright, but its not a guarantee.

mLewisLogic
A: 

http://codesynthesis.com/~boris/blog/2009/04/06/cxx-data-alignment-portability/

darelf
Thanks. I skimmed that before posting, but didn't pick up on the array aspect of the question.
Adam
+4  A: 

If you copy your array like this you should use

memcpy(pBuff,listOfFoos,sizeof(listOfFoos));

This will always work as long as you allocated pBuff to the same size. This way you are making no assumptions on padding and alignment at all.

Most compilers align a struct or class to the required alignment of the largest type included. In your case of chars that means no alignment and padding, but if you add a short for example your class would be 6 bytes large with one byte of padding added between the last char and your short.

nschmidt
+1  A: 

It all comes down to memory alignment. Typical 32-bit machines read or write 4 bytes of memory per attempt. This structure is safe from problems because it falls under that 4 bytes easily with no confusing padding issues.

Now if the structure was as such:

class foo {
   unsigned char a;
   unsigned char b;
   unsigned char c;
   unsigned int i;
   unsigned int j;
}

Your coworkers logic would probably lead to

memcpy(pBuff,listOfFoos,11*SOME_NUM);

(3 char's = 3 bytes, 2 ints = 2*4 bytes, so 3 + 8)

Unfortunately, due to padding the structure actually takes up 12 bytes. This is because you cannot fit three char's and an int into that 4 byte word, and so there's one byte of padded space there which pushes the int into it's own word. This becomes more and more of a problem the more diverse the data types become.

Afcrowe
+2  A: 

I think the reason that this works because all of the fields in the structure are char which align one. If there is at least one field that does not align 1, the alignment of the structure/class will not be 1 (the alignment will depends on the field order and alignment).

Let see some example:

#include <stdio.h>
#include <stddef.h>

typedef struct {
    unsigned char a;
    unsigned char b;
    unsigned char c;
} Foo;
typedef struct {
    unsigned short i;
    unsigned char  a;
    unsigned char  b;
    unsigned char  c;
} Bar;
typedef struct { Foo F[5]; } F_B;
typedef struct { Bar B[5]; } B_F;


#define ALIGNMENT_OF(t) offsetof( struct { char x; t test; }, test )

int main(void) {
    printf("Foo:: Size: %d; Alignment: %d\n", sizeof(Foo), ALIGNMENT_OF(Foo));
    printf("Bar:: Size: %d; Alignment: %d\n", sizeof(Bar), ALIGNMENT_OF(Bar));
    printf("F_B:: Size: %d; Alignment: %d\n", sizeof(F_B), ALIGNMENT_OF(F_B));
    printf("B_F:: Size: %d; Alignment: %d\n", sizeof(B_F), ALIGNMENT_OF(B_F));
}

When executed, the result is:

Foo:: Size: 3; Alignment: 1
Bar:: Size: 6; Alignment: 2
F_B:: Size: 15; Alignment: 1
B_F:: Size: 30; Alignment: 2

You can see that Bar and F_B has alignment 2 so that its field i will be properly aligned. You can also see that Size of Bar is 6 and not 5. Similarly, the size of B_F (5 of Bar) is 30 and not 25.

So, if you is a hard code instead of sizeof(...), you will get a problem here.

Hope this helps.

NawaMan
looks great, unfortunately the anonymous struct inside the offsetof call does not compile in msvc 2010
ufotds
+2  A: 

For situations where stuff like this is used, and I can't avoid it. I try to make the compilation break when the presumptions no longer hold. I use something like the following (Or http://www.boost.org/doc/libs/1_42_0/doc/html/boost_staticassert.html if the situation allows):

static_assert(sizeof(foo) <= 3);

// Macro for "static-assert" (only usefull on compile-time constant expressions)
#define static_assert(exp)           static_assert_II(exp, __LINE__)
// Macro used by static_assert macro (don't use directly)
#define static_assert_II(exp, line)  static_assert_III(exp, line)
// Macro used by static_assert macro (don't use directly)
#define static_assert_III(exp, line) enum static_assertion##line{static_assert_line_##line = 1/(exp)}
S.C. Madsen