sparse preprocessing confused

sparse preprocessing confused

[Randy Dunlap]

Although this may just be a case of "it's fugly, kill it".

In net/core/skbuff.c (2.6.21-rc7-mm1), we have:

	memcpy(data + nhead, skb->head,
#ifdef NET_SKBUFF_DATA_USES_OFFSET
		skb->tail);
#else
		skb->tail - skb->head);
#endif

and sparse says:

net/core/skbuff.c:651:2: error: unmatched #else within stream
net/core/skbuff.c:653:2: error: unmatched #endif in stream
net/core/skbuff.c:648:2: error: Expected ) in expression
net/core/skbuff.c:648:2: error: got #
net/core/skbuff.c:648:2: error: Expected ) in function call
net/core/skbuff.c:648:2: error: got #
net/core/skbuff.c:648:2: error: Expected ; at end of statement
net/core/skbuff.c:648:2: error: got __ret
net/core/skbuff.c:652:24: error: Expected ; at end of statement
net/core/skbuff.c:652:24: error: got )
net/core/skbuff.c:648:2: error: undefined identifier 'else'
net/core/skbuff.c:652:13: error: incompatible types for operation (-)
net/core/skbuff.c:652:13:    left side has type unsigned int [unsigned] [usertype] tail
net/core/skbuff.c:652:13:    right side has type unsigned char *head

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: sparse preprocessing confused

[Morten Welinder]

On 4/25/07, Randy Dunlap <rdunlap@xenotime.net> wrote:
> Although this may just be a case of "it's fugly, kill it".
>
> In net/core/skbuff.c (2.6.21-rc7-mm1), we have:
>
>         memcpy(data + nhead, skb->head,
> #ifdef NET_SKBUFF_DATA_USES_OFFSET
>                 skb->tail);
> #else
>                 skb->tail - skb->head);
> #endif

If memcpy happens to be a macro, that is not valid C.

M.

Re: sparse preprocessing confused

[Neil Booth]

Morten Welinder wrote:-

> On 4/25/07, Randy Dunlap <rdunlap@xenotime.net> wrote:
> >Although this may just be a case of "it's fugly, kill it".
> >
> >In net/core/skbuff.c (2.6.21-rc7-mm1), we have:
> >
> >        memcpy(data + nhead, skb->head,
> >#ifdef NET_SKBUFF_DATA_USES_OFFSET
> >                skb->tail);
> >#else
> >                skb->tail - skb->head);
> >#endif
> 
> If memcpy happens to be a macro, that is not valid C.

It's undefined rather than invalid.  GCC makes an effort to
do "what you'd expect".

Neil.

Re: sparse preprocessing confused

[Chris Wedgwood]

On Thu, Apr 26, 2007 at 11:38:52PM +0900, Neil Booth wrote:

> > >        memcpy(data + nhead, skb->head,
> > >#ifdef NET_SKBUFF_DATA_USES_OFFSET
> > >                skb->tail);
> > >#else
> > >                skb->tail - skb->head);
> > >#endif
> >
> > If memcpy happens to be a macro, that is not valid C.
>
> It's undefined rather than invalid.  GCC makes an effort to
> do "what you'd expect".

I think I prefer sparse breaking on things like this (so we can find
such uglies and correct them.).

Re: sparse preprocessing confused

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

Chris Wedgwood wrote:
> On Thu, Apr 26, 2007 at 11:38:52PM +0900, Neil Booth wrote:
>>>>        memcpy(data + nhead, skb->head,
>>>> #ifdef NET_SKBUFF_DATA_USES_OFFSET
>>>>                skb->tail);
>>>> #else
>>>>                skb->tail - skb->head);
>>>> #endif
>>> If memcpy happens to be a macro, that is not valid C.
>> It's undefined rather than invalid.  GCC makes an effort to
>> do "what you'd expect".
> 
> I think I prefer sparse breaking on things like this (so we can find
> such uglies and correct them.).

I tend to agree; however, if GCC accepts the code, I would prefer to have
Sparse parse it and warn, rather than failing to parse it.  I would gladly
take a clean patch that allows this code to work the same way GCC does.  Bonus
if the patch supplies a -Wpreprocessor-conditional-in-macro-arguments or
similar, and I wouldn't object to having such a warning on by default.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]