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

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

On 20/01/14 12:30, Dan Carpenter wrote:
> Ah.  From so metag is a new arch and not a compiler like the changelog
> says.
> 
> On Mon, Jan 20, 2014 at 11:56:47AM +0000, James Hogan wrote:
>> struct a {
>> 	struct b {
>> 		unsigned int x;
>> 		unsigned short y;
>> 	} x;
>> 	unsigned short y;
>> } __packed;
> 
> This is not the code we are discussing.  It should look like:
> 
> struct a {
> 	union {
> 		short x;
> 		short y;
> 	}
> 	short z;
> };
> 
> Any normal person would assume that sizeof(struct a) would be 4 but
> apparently on metag it is 8.  That totally defeats the point of using
> a union in the first place.  It's easy enough to add a __packed to the
> lustre declaration but I expect this to cause an endless stream of bugs.
> 
> It it is really stupid.

I agree completely (and did request this be changed when I first found
out about it, but since it's an ABI issue it was really too late).
That's why I'm not actively pushing for every case to be fixed unless
it's in generic code that actually affects metag.

Cheers
James