Re: [PATCH] drivers: staging: lustre: lustre: include: add "__attribute__((packed))" for the related union

[James Hogan <james.hogan@imgtec.com>]

Hi Chen,

On 19/01/14 10:07, Chen Gang wrote:
> BTW: this patch is related with another patch which is discussing (so I
> have cc that patch to you and Greg too): "if we could sure that it is a
> compiler's feature issue, we will skip this patch".

If you're referring to the #pragma pack portability issue then this
issue is unrelated since it doesn't use #pragma pack.

The issue is *not* that the compiler is defectively failing to pack
nested structs. Doing that would be utterly broken since it would change
the layout of the same struct depending on where it is placed in the
program, Consider this example (which uses a nested struct rather than
union to demonstrate the point):

struct a {
	struct b {
		unsigned int x;
		unsigned short y;
	} x;
	unsigned short y;
} __packed;

Both ABIs behave the same here:

Arch	sizeof(struct b)	sizeof(struct a)
x86_64	8			10
metag	8			10

If struct b is made __packed, again both ABIs behave differently in the
same way:

Arch	sizeof(struct b)	sizeof(struct a)
x86_64	6			8
metag	6			8

The issue is that C compiler ABIs may (and unfortunately metag ABI does)
pack structs and unions to at least 4 bytes, even if no members of the
struct or union are that large, which means that the nested struct/union
should be __packed too to portably ensure it's the expected size.

Cheers
James