Re: sparse vs. skbuff.h

[Eric Dumazet <eric.dumazet@gmail.com>]

Vegard Nossum a écrit :
> 2009/11/16 Eric Dumazet <eric.dumazet@gmail.com>:
>> Johannes Berg a écrit :
>>> On Mon, 2009-11-16 at 20:21 +0100, Johannes Berg wrote:
>>>> commit 14d18a81b5171d4433e41129619c75748b4f4d26
>>>> Author: Eric Dumazet <eric.dumazet@gmail.com>
>>>> Date:   Thu Oct 29 00:10:37 2009 +0000
>>>>
>>>>     net: fix kmemcheck annotations
>>>>
>>>>
>>>> broke sparse endian checks on everything that includes skbuff.h because
>>>> the first and only (because it's an error) thing sparse now reports is
>>>> this:
>>>>
>>>> include/linux/skbuff.h:357:41: error: invalid bitfield specifier for type restricted __be16.
>>> Simply changing from
>>>       __be16 protocol:16;
>>> to
>>>       __be16 protocol;
>>>
>>> but leaving it inside the kmemcheck annotation seems to do the right
>>> thing. Except of course that kmemcheck will not properly check it now.
>>> Maybe those annotations should simply be made to have no impact on
>>> struct padding instead?
>>>
>> Hmm, I have really no idea of what is the right way to fix this stuff.
>>
>> Last time I did adding a non bitfield element inside the begin/end annotations,
>> I was flamed, because a bitfield is a bitfield, not a char/short
>>
>> http://www.spinics.net/lists/netdev/msg108803.html
>>
>> Now sparse is complaining... What will be the next story ?
> 
> If by "I was flamed" you are referring to my reply:
> 
> http://www.spinics.net/lists/netdev/msg108825.html
> 
> then I am really sorry, because I had no intentions to insult you. In
> fact, I am grateful that you are finding bugs and telling me about
> them But I should also be allowed to disagree with a patch if I truly
> believe it is the wrong thing to do.

Oh well, maybe flamed is not the right word Vegard, and I did not feel
being insulted at all ! Sorry if the tone of my mail was misleading.

> 
> For the issue in question: If the variable is turned into a
> non-bitfield (as Johannes suggested), it would be fine, because now
> GCC won't emit masking operations (AND, OR) when initializing it, but
> a regular MOV. Also, the struct annotations do not by themselves do
> anything, but they are used by kmemcheck_annotate_bitfield().
> 
> In other words, I think the right thing to do is to turn it into a
> non-bitfield and move it _outside_ the bitfield annotation. Johannes,
> can you make the patch and let us have a look? In the meantime I will
> submit the patch that fixes the extraneous field padding in
> KMEMCHECK=n kernels.
> 

Thanks