views:

124

answers:

5

So, I'm coding some packet structures (Ethernet, IP, etc) and noticed that some of them are followed by attribute((packed)) which prevents the gcc compiler from attempting to add padding to them. This makes sense, because these structures are supposed to go onto the wire.

But then, I counted the words:

struct ether_header
{
  u_int8_t  ether_dhost[ETH_ALEN];  /* destination eth addr */
  u_int8_t  ether_shost[ETH_ALEN];  /* source ether addr */
  u_int16_t ether_type;          /* packet type ID field */
} __attribute__ ((packed));

This is copied from a site, but my code also uses 2 uint8_t and 1 uint16_t. This adds up to two words (4 bytes).

Depending on the source, the system prefers that structures be aligned according to multiples of 4,8, or even 16 bits. So, I don't see why the attribute((packed)) is necessary, as afaik this shouldn't get packed.

Also, why the double brackets ((packed)) why not use one pair?

+5  A: 

If your structure is already a multiple of the right size, then no, the __attribute__((packed)) isn't strictly necessary, but it's still a good idea, in case your structure size ever changes for any reason. If you add/delete fields, or change ETH_ALEN, you'll still want __attribute__((packed)).

I believe the double parentheses are needed to make it easy to make your code compatible with non-gcc compilers. By using them, you can then just do this:

#define __attribute__(x)

And then all attributes that you specify will disappear. The extra parentheses mean there is only one argument passed to the macro (instead of one or more), regardless of how many attributes you specify, and your compiler does not need to support variadic macros.

Adam Rosenfield
I forgot that the uint8_t's were arrays; silly me, but good call on planning for change.
Lanissum
+2  A: 

Although your system may prefer some specific alignment, other systems might not. Even if the __attribute__((packed)) has no effect, it's a good touch of paranoia.

As for why it's double-parenthesis, this GCC-specific extension requires double parenthesis. Single parenthesis will result in an error.

bdonlan
Excellent point. Consider a 64-bit or future 128-bit system.
Zan Lynx
A: 

packed refers to the padding/alignment inside the structure, not the alignment of the structure. For instance

struct {
  char x;
  int y;
}

Most compilers will allocate y at offset 4 unless you declare the struct as packed (in which case y will get allocated at an offset of 1).

Keith Randall
using packed on such a structure is a very bad idea since accesses to y will then trap on a number of platforms.
Peeter Joot
More generally, depending on a C structure to represent a platform-independent data layout is asking for trouble. packed just trades one trouble for another.
Keith Randall
+1  A: 
in win32, you can do like this: 
#pragma pack(push) //save current status 
#pragma pack(4)//set following as 4 aligned 
struct test 
{ 
char m1; 
double m4; 
int m3; 
}; 
#pragma pack(pop) //restore 
Macroideal
Those pragmas work in GCC as well.
Zan Lynx
A: 

For this structure, even if ETH_ALEN is an odd number, you have two of them, so the uint16 variable will neccessarily be at a two or zero byte offset, and the packed won't do anything. Depending on packed is a bad idea for portability, because the mechanism for packing aren't portable and if you use them you may have to byte copy in and out of your member variables to avoid misalignment exceptions on platforms that this matter for.

Peeter Joot