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

[Dan Carpenter]

On Sat, Jan 18, 2014 at 06:26:10PM +0800, Chen Gang wrote:
> On 01/18/2014 06:05 PM, Dan Carpenter wrote:
> > On Sat, Jan 18, 2014 at 05:50:34PM +0800, Chen Gang wrote:
> >> Unfortunately, not all compilers assumes the structures within a pack
> >> region also need be packed (e.g. metag), so need add a pack explicitly
> >> to satisfy all compilers.
> >>
> >> The related error (under metag with allmodconfig):
> >>
> >>     CC [M]  drivers/staging/lustre/lustre/lov/lov_pack.o
> >>   drivers/staging/lustre/lustre/lov/lov_pack.c: In function 'lov_getstripe':
> >>   drivers/staging/lustre/lustre/lov/lov_pack.c:630: error: duplicate case value
> >>   drivers/staging/lustre/lustre/lov/lov_pack.c:630: error: previously used here
> >>
> >> And originally, all related code used "__attribute__((packed))", so
> >> still use it instead of '__packed'.
> > 
> > Use __packed.  Then at least one line will be correct which is better
> > than nothing.
> > 
> 
> Hmm... but that will break the 'consistency' (which is not quite good
> for readers).
> 
> For me, it will be better to provide another patch to change all
> "__attribute__((packed))" to "__packed" within this file.
> 
> What about your ideas?
> 

In the end, it's not something we care about enough to ask you to redo
the patch.  But what I'm saying is that you should prefer kernel style
over local style.  We'll fix the surrounding lines later.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/