Re: [PATCH] mac80211: check endianness/types in sparse runs

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* Re: [PATCH] mac80211: check endianness/types in sparse runs
       [not found] ` <20080220195913.GE21139@uranus.ravnborg.org>
@ 2008-02-21 12:42   ` Johannes Berg
  2008-02-21 20:01     ` Harvey Harrison
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Johannes Berg @ 2008-02-21 12:42 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: John Linville, linux-wireless, Stefano Brivio, Al Viro,
	Linux Kernel list

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

>> [patch doing CHECKFLAGS += -D__CHECK_ENDIAN__ in the
>> net/mac80211/Makefile]

> I would prefer it to be kernel wide enabled.
> Tried a defconfig build.

Hm. I tend to think there was a reason for this, since this is actually
explicitly disabled by include/linux/types.h:

#ifdef __CHECKER__
#define __bitwise__ __attribute__((bitwise))
#else 
#define __bitwise__
#endif

#ifdef __CHECK_ENDIAN__
#define __bitwise __bitwise__
#else
#define __bitwise
#endif

The commit that introduced __CHECK_ENDIAN__ was
af4ca457eaf2d6682059c18463eb106e2ce58198 ("gfp_t: infrastructure") but
it doesn't say anything about the rationale for it.

> When I enabled __CHECK_ENDIAN I got:
> 8 files with > 100 warnings
> 14 files with 10 to 99 warnings.
> 
> So nothing that should scare a kernel hacker...
> 
> warnings without: 1686
> warnings with: 2788
> 
> OK - thats a lot, but then fixing 8 files will significantly
> reduce this.

I recently ran sparse on my config and was surprised by the number of
warnings. Then again, something in mmzone.h or so generated billions of
them...

In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
least on the wireless code (just caught another bug with it...)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
@ 2008-02-21 20:01     ` Harvey Harrison
  2008-02-21 22:32       ` __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs] Sam Ravnborg
  2008-02-21 20:06     ` [PATCH] mac80211: check endianness/types in sparse runs Sam Ravnborg
  2008-02-21 21:16     ` Al Viro
  2 siblings, 1 reply; 11+ messages in thread
From: Harvey Harrison @ 2008-02-21 20:01 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Al Viro, Linux Kernel list

On Thu, 2008-02-21 at 13:42 +0100, Johannes Berg wrote:
> >> [patch doing CHECKFLAGS += -D__CHECK_ENDIAN__ in the
> >> net/mac80211/Makefile]
> 
> > I would prefer it to be kernel wide enabled.
> > Tried a defconfig build.
> 
> Hm. I tend to think there was a reason for this, since this is actually
> explicitly disabled by include/linux/types.h:
> 
> #ifdef __CHECKER__
> #define __bitwise__ __attribute__((bitwise))
> #else 
> #define __bitwise__
> #endif
> 
> #ifdef __CHECK_ENDIAN__
> #define __bitwise __bitwise__
> #else
> #define __bitwise
> #endif
> 

This seems somewhat suspect.

> I recently ran sparse on my config and was surprised by the number of
> warnings. Then again, something in mmzone.h or so generated billions of
> them...

Potentially expensive pointer subtraction 640:....

Patch in -mm.

> 
> In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> least on the wireless code (just caught another bug with it...)
> 

I'd love to have it too, but there are so many trivial warnings that
clutter up valid warnings that it is prohibitive.  I'm working on
reducing the noise level a bit for 2.6.26, we'll see about turning
it on then.

Harvey


^ permalink raw reply	[flat|nested] 11+ messages in thread

* __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]
  2008-02-21 20:01     ` Harvey Harrison
@ 2008-02-21 22:32       ` Sam Ravnborg
  2008-02-21 22:38         ` Al Viro
  0 siblings, 1 reply; 11+ messages in thread
From: Sam Ravnborg @ 2008-02-21 22:32 UTC (permalink / raw)
  To: Harvey Harrison, Al Viro
  Cc: Johannes Berg, John Linville, linux-wireless, Stefano Brivio,
	Al Viro, Linux Kernel list

Al - can I ask you to explain the difference between __bitwise
and __bitwise__ as used by the kernel.

Se below for a bit of background info.

Thanks,
	Sam

On Thu, Feb 21, 2008 at 12:01:31PM -0800, Harvey Harrison wrote:
> On Thu, 2008-02-21 at 13:42 +0100, Johannes Berg wrote:
> > >> [patch doing CHECKFLAGS += -D__CHECK_ENDIAN__ in the
> > >> net/mac80211/Makefile]
> > 
> > > I would prefer it to be kernel wide enabled.
> > > Tried a defconfig build.
> > 
> > Hm. I tend to think there was a reason for this, since this is actually
> > explicitly disabled by include/linux/types.h:
> > 
> > #ifdef __CHECKER__
> > #define __bitwise__ __attribute__((bitwise))
> > #else 
> > #define __bitwise__
> > #endif
> > 
> > #ifdef __CHECK_ENDIAN__
> > #define __bitwise __bitwise__
> > #else
> > #define __bitwise
> > #endif
> > 
> 
> This seems somewhat suspect.
> 
> > I recently ran sparse on my config and was surprised by the number of
> > warnings. Then again, something in mmzone.h or so generated billions of
> > them...
> 
> Potentially expensive pointer subtraction 640:....
> 
> Patch in -mm.
> 
> > 
> > In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> > least on the wireless code (just caught another bug with it...)
> > 
> 
> I'd love to have it too, but there are so many trivial warnings that
> clutter up valid warnings that it is prohibitive.  I'm working on
> reducing the noise level a bit for 2.6.26, we'll see about turning
> it on then.
> 
> Harvey
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]
  2008-02-21 22:32       ` __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs] Sam Ravnborg
@ 2008-02-21 22:38         ` Al Viro
  0 siblings, 0 replies; 11+ messages in thread
From: Al Viro @ 2008-02-21 22:38 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: Harvey Harrison, Al Viro, Johannes Berg, John Linville,
	linux-wireless, Stefano Brivio, Linux Kernel list

On Thu, Feb 21, 2008 at 11:32:39PM +0100, Sam Ravnborg wrote:
> Al - can I ask you to explain the difference between __bitwise
> and __bitwise__ as used by the kernel.

__bitwise__ - to be used for relatively compact stuff (gfp_t, etc.) that
is mostly warning-free and is supposed to stay that way.  Warnings will
be generated without __CHECK_ENDIAN__.

__bitwise - noisy stuff; in particular, __le*/__be* are that.  We really
don't want to drown in noise unless we'd explicitly asked for it.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
  2008-02-21 20:01     ` Harvey Harrison
@ 2008-02-21 20:06     ` Sam Ravnborg
  2008-02-21 20:09       ` Johannes Berg
  2008-02-21 21:16     ` Al Viro
  2 siblings, 1 reply; 11+ messages in thread
From: Sam Ravnborg @ 2008-02-21 20:06 UTC (permalink / raw)
  To: Johannes Berg
  Cc: John Linville, linux-wireless, Stefano Brivio, Al Viro,
	Linux Kernel list

On Thu, Feb 21, 2008 at 01:42:56PM +0100, Johannes Berg wrote:
> >> [patch doing CHECKFLAGS += -D__CHECK_ENDIAN__ in the
> >> net/mac80211/Makefile]
> 
> > I would prefer it to be kernel wide enabled.
> > Tried a defconfig build.
> 
> Hm. I tend to think there was a reason for this, since this is actually
> explicitly disabled by include/linux/types.h:
> 
> #ifdef __CHECKER__
> #define __bitwise__ __attribute__((bitwise))
> #else 
> #define __bitwise__
> #endif
> 
> #ifdef __CHECK_ENDIAN__
> #define __bitwise __bitwise__
> #else
> #define __bitwise
> #endif
> 
> The commit that introduced __CHECK_ENDIAN__ was
> af4ca457eaf2d6682059c18463eb106e2ce58198 ("gfp_t: infrastructure") but
> it doesn't say anything about the rationale for it.
> 
> > When I enabled __CHECK_ENDIAN I got:
> > 8 files with > 100 warnings
> > 14 files with 10 to 99 warnings.
> > 
> > So nothing that should scare a kernel hacker...
> > 
> > warnings without: 1686
> > warnings with: 2788
> > 
> > OK - thats a lot, but then fixing 8 files will significantly
> > reduce this.
> 
> I recently ran sparse on my config and was surprised by the number of
> warnings. Then again, something in mmzone.h or so generated billions of
> them...
> 
> In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> least on the wireless code (just caught another bug with it...)

I should then add support for something like:

checkflags-y := -D__CHECK_ENDIAN__

to match the style of the rest.

I do not like all the buried in assumption about the
global variables.


But I would prefer that someone would spend a few days looking into the
warnings generated with sparse.
Dedicating a few hours/day in a week could help a lot here if
one understand the warnings.

What I see is that people mindlessly add code that introduce
new sparse warnings without noticing simply due to the
massive amount of warnings generated.

	Sam

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 20:06     ` [PATCH] mac80211: check endianness/types in sparse runs Sam Ravnborg
@ 2008-02-21 20:09       ` Johannes Berg
  2008-02-21 20:25         ` Harvey Harrison
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2008-02-21 20:09 UTC (permalink / raw)
  To: Sam Ravnborg
  Cc: John Linville, linux-wireless, Stefano Brivio, Al Viro,
	Linux Kernel list

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


> > In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> > least on the wireless code (just caught another bug with it...)
> 
> I should then add support for something like:
> 
> checkflags-y := -D__CHECK_ENDIAN__
> 
> to match the style of the rest.
> 
> I do not like all the buried in assumption about the
> global variables.

That might be good. I retract my patch for now then.

> But I would prefer that someone would spend a few days looking into the
> warnings generated with sparse.

I did spend a bit of time running it on all the tree, but wireless,
where I do most work, is clean. I have some code in sound/ that
generates warnings and some drivers/macintosh/ code I worked on too, but
right now I don't want to pick up too many changes all over the tree
into my patches tree.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 20:09       ` Johannes Berg
@ 2008-02-21 20:25         ` Harvey Harrison
  2008-02-21 20:29           ` Johannes Berg
  0 siblings, 1 reply; 11+ messages in thread
From: Harvey Harrison @ 2008-02-21 20:25 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Al Viro, Linux Kernel list

On Thu, 2008-02-21 at 21:09 +0100, Johannes Berg wrote:
> > > In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> > > least on the wireless code (just caught another bug with it...)
> > 
> > I should then add support for something like:
> > 
> > checkflags-y := -D__CHECK_ENDIAN__
> > 
> > to match the style of the rest.
> > 
> > I do not like all the buried in assumption about the
> > global variables.
> 
> That might be good. I retract my patch for now then.
> 
> > But I would prefer that someone would spend a few days looking into the
> > warnings generated with sparse.
> 
> I did spend a bit of time running it on all the tree, but wireless,
> where I do most work, is clean. 

Clean, or did you specifically mean bitwise-clean?

drivers/net/wireless/ipw2100.c:2837:7: warning: symbol 'i' shadows an earlier one
drivers/net/wireless/ipw2100.c:2760:9: originally declared here
drivers/net/wireless/ipw2100.c:7873:15: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2100.c:187:12: originally declared here
drivers/net/wireless/ipw2100.c:7937:11: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2100.c:187:12: originally declared here
drivers/net/wireless/ipw2100.c:7985:11: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2100.c:187:12: originally declared here
drivers/net/wireless/ipw2100.c:8253:13: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2100.c:8253:13: originally declared here
drivers/net/wireless/ipw2100.c:8253:13: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2100.c:8253:13: originally declared here
drivers/net/wireless/ipw2100.c:8253:13: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2100.c:8253:13: originally declared here
drivers/net/wireless/ipw2100.c:1929:15: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:1929:15:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:1929:15:    got int *<noident>
drivers/net/wireless/ipw2100.c:1937:69: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:1937:69:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:1937:69:    got int *<noident>
drivers/net/wireless/ipw2100.c:1945:60: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:1945:60:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:1945:60:    got int *<noident>
drivers/net/wireless/ipw2100.c:1952:65: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:1952:65:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:1952:65:    got int *<noident>
drivers/net/wireless/ipw2100.c:4071:66: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:4071:66:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:4071:66:    got int *<noident>
drivers/net/wireless/ipw2100.c:4078:15: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:4078:15:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:4078:15:    got int *<noident>
drivers/net/wireless/ipw2100.c:4084:60: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:4084:60:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:4084:60:    got int *<noident>
drivers/net/wireless/ipw2100.c:5275:6: warning: symbol 'ipw2100_queues_initialize' was not declared. Should it be static?
drivers/net/wireless/ipw2100.c:5282:6: warning: symbol 'ipw2100_queues_free' was not declared. Should it be static?
drivers/net/wireless/ipw2100.c:5289:5: warning: symbol 'ipw2100_queues_allocate' was not declared. Should it be static?
drivers/net/wireless/ipw2100.c:7146:66: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:7146:66:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:7146:66:    got int *<noident>
drivers/net/wireless/ipw2100.c:8322:20: warning: incorrect type in argument 4 (different signedness)
drivers/net/wireless/ipw2100.c:8322:20:    expected unsigned int [usertype] *len
drivers/net/wireless/ipw2100.c:8322:20:    got int *<noident>
drivers/net/wireless/ipw2200.c:833:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:877:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:921:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:966:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:1002:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:1037:6: warning: symbol 'led' shadows an earlier one
drivers/net/wireless/ipw2200.c:93:12: originally declared here
drivers/net/wireless/ipw2200.c:1806:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/ipw2200.c:87:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_x' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:4253:12: warning: symbol '_y' shadows an earlier one
drivers/net/wireless/ipw2200.c:4253:12: originally declared here
drivers/net/wireless/ipw2200.c:6211:7: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/ipw2200.c:87:12: originally declared here
drivers/net/wireless/ipw2200.c:6327:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/ipw2200.c:87:12: originally declared here
drivers/net/wireless/ipw2200.c:6793:5: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2200.c:88:12: originally declared here
drivers/net/wireless/ipw2200.c:7888:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/ipw2200.c:87:12: originally declared here
drivers/net/wireless/ipw2200.c:8662:5: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/ipw2200.c:87:12: originally declared here
drivers/net/wireless/ipw2200.c:9665:6: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2200.c:88:12: originally declared here
drivers/net/wireless/ipw2200.c:9723:6: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2200.c:88:12: originally declared here
drivers/net/wireless/ipw2200.c:9829:6: warning: symbol 'mode' shadows an earlier one
drivers/net/wireless/ipw2200.c:88:12: originally declared here
drivers/net/wireless/ipw2200.c:10325:7: warning: symbol 'remaining_bytes' shadows an earlier one
drivers/net/wireless/ipw2200.c:10193:6: originally declared here
drivers/net/wireless/strip.c:490:16: error: bad constant expression
drivers/net/wireless/arlan-main.c:969:48: warning: incorrect type in initializer (different address spaces)
drivers/net/wireless/arlan-main.c:969:48:    expected struct arlan_shmem volatile [noderef] <asn:2>*arlan
drivers/net/wireless/arlan-main.c:969:48:    got struct arlan_shmem *<noident>
drivers/net/wireless/arlan-main.c:1088:11: warning: incorrect type in assignment (different address spaces)
drivers/net/wireless/arlan-main.c:1088:11:    expected struct arlan_shmem [noderef] <asn:2>*card
drivers/net/wireless/arlan-main.c:1088:11:    got void *<noident>
drivers/net/wireless/arlan-main.c:1776:19: warning: symbol 'arlan_probe' was not declared. Should it be static?
drivers/net/wireless/rtl8180_dev.c:138:42: warning: Using plain integer as NULL pointer
drivers/net/wireless/rtl8187_dev.c:116:41: warning: Using plain integer as NULL pointer
drivers/net/wireless/orinoco.c:3232:2: warning: context imbalance in 'orinoco_ioctl_getnick' - unexpected unlock
drivers/net/wireless/airo.c:3800:8: warning: symbol 'i' shadows an earlier one
drivers/net/wireless/airo.c:3711:6: originally declared here
drivers/net/wireless/airo.c:3807:8: warning: symbol 'i' shadows an earlier one
drivers/net/wireless/airo.c:3711:6: originally declared here
drivers/net/wireless/airo.c:3821:7: warning: symbol 'i' shadows an earlier one
drivers/net/wireless/airo.c:3711:6: originally declared here
drivers/net/wireless/airo.c:2092:14: warning: incorrect type in initializer (different signedness)
drivers/net/wireless/airo.c:2092:14:    expected unsigned int [usertype] *fids
drivers/net/wireless/airo.c:2092:14:    got int *<noident>
drivers/net/wireless/airo.c:2116:14: warning: incorrect type in initializer (different signedness)
drivers/net/wireless/airo.c:2116:14:    expected unsigned int [usertype] *fids
drivers/net/wireless/airo.c:2116:14:    got int *<noident>
drivers/net/wireless/airo.c:2157:14: warning: incorrect type in initializer (different signedness)
drivers/net/wireless/airo.c:2157:14:    expected unsigned int [usertype] *fids
drivers/net/wireless/airo.c:2157:14:    got int *<noident>
drivers/net/wireless/airo.c:2181:14: warning: incorrect type in initializer (different signedness)
drivers/net/wireless/airo.c:2181:14:    expected unsigned int [usertype] *fids
drivers/net/wireless/airo.c:2181:14:    got int *<noident>
drivers/net/wireless/airo.c:3612:6: warning: symbol 'mpi_receive_802_11' was not declared. Should it be static?
drivers/net/wireless/atmel.c:3174:6: warning: symbol 'atmel_join_bss' was not declared. Should it be static?
drivers/net/wireless/ath5k/base.c:1682:1: warning: context imbalance in 'ath5k_tasklet_rx' - different lock contexts for basic block
drivers/net/wireless/ath5k/hw.c:3764:10: warning: cast truncates bits from constant value (ffffffea becomes 0)
drivers/net/wireless/b43legacy/phy.c:1298:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/b43legacy/phy.c:1298:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/b43legacy/phy.c:1298:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/b43legacy/pio.c:140:16: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_main.c:3232:2: warning: context imbalance in 'bcm43xx_periodic_work_handler' - different lock contexts for basic block
drivers/net/wireless/bcm43xx/bcm43xx_phy.c:1470:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_phy.c:1470:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_phy.c:1470:31: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_phy.c:94:6: warning: context imbalance in 'bcm43xx_raw_phy_lock' - different lock contexts for basic block
drivers/net/wireless/bcm43xx/bcm43xx_phy.c:120:4: warning: context imbalance in 'bcm43xx_raw_phy_unlock' - unexpected unlock
drivers/net/wireless/bcm43xx/bcm43xx_leds.c:38:20: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_leds.c:77:20: warning: potentially expensive pointer subtraction
drivers/net/wireless/bcm43xx/bcm43xx_pio.c:140:16: warning: potentially expensive pointer subtraction
drivers/net/wireless/hostap/hostap_80211_rx.c:128:34: warning: symbol 'hdr' shadows an earlier one
drivers/net/wireless/hostap/hostap_80211_rx.c:67:29: originally declared here
drivers/net/wireless/hostap/hostap_80211_rx.c:150:32: warning: symbol 'hdr' shadows an earlier one
drivers/net/wireless/hostap/hostap_80211_rx.c:67:29: originally declared here
drivers/net/wireless/hostap/hostap_ap.c:1903:18: warning: cast truncates bits from constant value (ffff3fff becomes 3fff)
drivers/net/wireless/hostap/hostap_main.c:597:5: warning: symbol 'hostap_80211_header_parse' was not declared. Should it be static?
drivers/net/wireless/hostap/hostap_hw.c:2838:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/hostap/hostap_hw.c:64:12: originally declared here
drivers/net/wireless/hostap/hostap_cs.c:583:3: warning: symbol 'ret' shadows an earlier one
drivers/net/wireless/hostap/hostap_cs.c:552:6: originally declared here
drivers/net/wireless/hostap/hostap_cs.c:585:3: warning: symbol 'ret' shadows an earlier one
drivers/net/wireless/hostap/hostap_cs.c:552:6: originally declared here
drivers/net/wireless/hostap/hostap_cs.c:665:3: warning: symbol 'ret' shadows an earlier one
drivers/net/wireless/hostap/hostap_cs.c:552:6: originally declared here
drivers/net/wireless/hostap/hostap_hw.c:2838:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/hostap/hostap_hw.c:64:12: originally declared here
drivers/net/wireless/hostap/hostap_hw.c:2838:6: warning: symbol 'channel' shadows an earlier one
drivers/net/wireless/hostap/hostap_hw.c:64:12: originally declared here
drivers/net/wireless/prism54/isl_ioctl.c:2052:20: warning: incorrect type in assignment (different address spaces)
drivers/net/wireless/prism54/isl_ioctl.c:2052:20:    expected void [noderef] <asn:1>*pointer
drivers/net/wireless/prism54/isl_ioctl.c:2052:20:    got char *[assigned] memptr
drivers/net/wireless/prism54/isl_ioctl.c:2071:20: warning: incorrect type in assignment (different address spaces)
drivers/net/wireless/prism54/isl_ioctl.c:2071:20:    expected void [noderef] <asn:1>*pointer
drivers/net/wireless/prism54/isl_ioctl.c:2071:20:    got char *[assigned] memptr
drivers/net/wireless/prism54/isl_ioctl.c:2993:25: warning: incorrect type in argument 2 (different signedness)
drivers/net/wireless/prism54/isl_ioctl.c:2993:25:    expected int enum oid_num_t *<noident>
drivers/net/wireless/prism54/isl_ioctl.c:2993:25:    got unsigned int *<noident>
drivers/net/wireless/prism54/islpci_hotplug.c:97:1: warning: symbol 'prism54_probe' was not declared. Should it be static?
drivers/net/wireless/prism54/islpci_hotplug.c:220:1: warning: symbol 'prism54_remove' was not declared. Should it be static?
drivers/net/wireless/prism54/islpci_hotplug.c:263:1: warning: symbol 'prism54_suspend' was not declared. Should it be static?
drivers/net/wireless/prism54/islpci_hotplug.c:286:1: warning: symbol 'prism54_resume' was not declared. Should it be static?
drivers/net/wireless/prism54/oid_mgt.c:424:15: warning: symbol '_data' shadows an earlier one
include/asm-generic/sections.h:7:13: originally declared here
drivers/net/wireless/prism54/oid_mgt.c:712:35: warning: incorrect type in argument 2 (different signedness)
drivers/net/wireless/prism54/oid_mgt.c:712:35:    expected int enum oid_num_t *l
drivers/net/wireless/prism54/oid_mgt.c:712:35:    got unsigned int *<noident>
drivers/net/wireless/rt2x00/rt2x00mac.c:217:13: warning: Using plain integer as NULL pointer
drivers/net/wireless/zd1211rw/zd_mac.c:363:40: warning: Using plain integer as NULL pointer
drivers/net/wireless/zd1211rw/zd_mac.c:392:42: warning: Using plain integer as NULL pointer
drivers/net/wireless/zd1211rw/zd_mac.c:606:42: warning: Using plain integer as NULL pointer
drivers/net/wireless/ray_cs.c:831:5: warning: symbol 'ray_dev_init' was not declared. Should it be static?
drivers/net/wireless/rndis_wlan.c:282:64: warning: symbol 'wpa_alg' was not declared. Should it be static?
drivers/net/wireless/rndis_wlan.c:284:2: warning: symbol 'wpa_cipher' was not declared. Should it be static?
drivers/net/wireless/rndis_wlan.c:286:22: warning: symbol 'wpa_key_mgmt' was not declared. Should it be static?
drivers/net/wireless/p54common.c:352:42: warning: Using plain integer as NULL pointer

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 20:25         ` Harvey Harrison
@ 2008-02-21 20:29           ` Johannes Berg
  2008-02-21 20:33             ` Harvey Harrison
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Berg @ 2008-02-21 20:29 UTC (permalink / raw)
  To: Harvey Harrison
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Al Viro, Linux Kernel list

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


> Clean, or did you specifically mean bitwise-clean?

bitwise-clean. But I don't do full-mac drivers so most of what you quote
I don't compile, and the mac80211-based drivers only have few problems.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 20:29           ` Johannes Berg
@ 2008-02-21 20:33             ` Harvey Harrison
  0 siblings, 0 replies; 11+ messages in thread
From: Harvey Harrison @ 2008-02-21 20:33 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Al Viro, Linux Kernel list

On Thu, 2008-02-21 at 21:29 +0100, Johannes Berg wrote:
> > Clean, or did you specifically mean bitwise-clean?
> 
> bitwise-clean. But I don't do full-mac drivers so most of what you quote
> I don't compile, and the mac80211-based drivers only have few problems.
> 

This wasn't meant as any sort of slight, actually I completely agree
that the current warning level means real stuff gets missed.

I've knocked off about a thousand lines from an X86_32 allyesconfig
sparse run this week, but the fruit is starting to get harder to reach.

Harvey


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
  2008-02-21 20:01     ` Harvey Harrison
  2008-02-21 20:06     ` [PATCH] mac80211: check endianness/types in sparse runs Sam Ravnborg
@ 2008-02-21 21:16     ` Al Viro
  2008-02-22 14:02       ` Johannes Berg
  2 siblings, 1 reply; 11+ messages in thread
From: Al Viro @ 2008-02-21 21:16 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Linux Kernel list

On Thu, Feb 21, 2008 at 01:42:56PM +0100, Johannes Berg wrote:
> >> [patch doing CHECKFLAGS += -D__CHECK_ENDIAN__ in the
> >> net/mac80211/Makefile]
> 
> > I would prefer it to be kernel wide enabled.
> > Tried a defconfig build.
> 
> Hm. I tend to think there was a reason for this, since this is actually
> explicitly disabled by include/linux/types.h:
> 
> #ifdef __CHECKER__
> #define __bitwise__ __attribute__((bitwise))
> #else 
> #define __bitwise__
> #endif
> 
> #ifdef __CHECK_ENDIAN__
> #define __bitwise __bitwise__
> #else
> #define __bitwise
> #endif
> 
> The commit that introduced __CHECK_ENDIAN__ was
> af4ca457eaf2d6682059c18463eb106e2ce58198 ("gfp_t: infrastructure") but
> it doesn't say anything about the rationale for it.
> 
> > When I enabled __CHECK_ENDIAN I got:
> > 8 files with > 100 warnings
> > 14 files with 10 to 99 warnings.
> > 
> > So nothing that should scare a kernel hacker...
> > 
> > warnings without: 1686
> > warnings with: 2788
> > 
> > OK - thats a lot, but then fixing 8 files will significantly
> > reduce this.
> 
> I recently ran sparse on my config and was surprised by the number of
> warnings. Then again, something in mmzone.h or so generated billions of
> them...
> 
> In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> least on the wireless code (just caught another bug with it...)

So build with make C=2 -D__CHECK_ENDIAN__ net/ieee80211/, etc. - it's not
that such a script would be tricky...

I've been shooting the endianness crap down for quite a while (see e.g.
drivers/net/* patches in 2.6.24 and 2.6.24-rc1; there's more in queue)
and it's getting better, but not enough to enable it unconditionally.

Note that it's not quite a janitor-level stuff; blind "fixing" of warnings
in that area is very likely to result in hidden bugs - you need serious
RTFS + RTFDatasheet + ask maintainer + trawl list archives for bug reports
in quite a few cases.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 21:16     ` Al Viro
@ 2008-02-22 14:02       ` Johannes Berg
  0 siblings, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2008-02-22 14:02 UTC (permalink / raw)
  To: Al Viro
  Cc: Sam Ravnborg, John Linville, linux-wireless, Stefano Brivio,
	Linux Kernel list

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


> > In any case, I would love to have __CHECK_ENDIAN__ enabled by default at
> > least on the wireless code (just caught another bug with it...)
> 
> So build with make C=2 -D__CHECK_ENDIAN__ net/ieee80211/, etc. - it's not
> that such a script would be tricky...

It's not that I don't know how to do it... it's that I don't want to
educate each person who gives me patches about sparse AND the check
endian stuff etc.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2008-02-22 14:05 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <1203503909.17534.14.camel@johannes.berg>
     [not found] ` <20080220195913.GE21139@uranus.ravnborg.org>
2008-02-21 12:42   ` [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
2008-02-21 20:01     ` Harvey Harrison
2008-02-21 22:32       ` __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs] Sam Ravnborg
2008-02-21 22:38         ` Al Viro
2008-02-21 20:06     ` [PATCH] mac80211: check endianness/types in sparse runs Sam Ravnborg
2008-02-21 20:09       ` Johannes Berg
2008-02-21 20:25         ` Harvey Harrison
2008-02-21 20:29           ` Johannes Berg
2008-02-21 20:33             ` Harvey Harrison
2008-02-21 21:16     ` Al Viro
2008-02-22 14:02       ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox