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

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

[Johannes Berg]

Make sure sparse checks endianness when run on mac80211.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
Sam, is this an OK use of makefiles or is there something else I should
be doing?

The only warnings this currently triggers are some signedness warnings
in the PID RC algorithm.

 net/mac80211/Makefile |    2 ++
 1 file changed, 2 insertions(+)

--- everything.orig/net/mac80211/Makefile	2008-02-19 18:35:28.151573893 +0100
+++ everything/net/mac80211/Makefile	2008-02-19 18:41:10.491563910 +0100
@@ -45,3 +45,5 @@ mac80211-$(CONFIG_MAC80211_RC_PID) += $(
 
 # Modular rate algorithms are assigned to mac80211-m - make separate modules
 obj-m += $(mac80211-m)
+
+CHECKFLAGS += -D__CHECK_ENDIAN__

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

[Sam Ravnborg]

On Wed, Feb 20, 2008 at 11:38:29AM +0100, Johannes Berg wrote:
> Make sure sparse checks endianness when run on mac80211.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> ---
> Sam, is this an OK use of makefiles or is there something else I should
> be doing?

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

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.

	Sam

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

[Pavel Roskin]

On Wed, 2008-02-20 at 20:59 +0100, Sam Ravnborg wrote:
> On Wed, Feb 20, 2008 at 11:38:29AM +0100, Johannes Berg wrote:
> > Make sure sparse checks endianness when run on mac80211.
> > 
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> > ---
> > Sam, is this an OK use of makefiles or is there something else I should
> > be doing?
> 
> I would prefer it to be kernel wide enabled.
> Tried a defconfig build.

I cannot agree more.  It's not like endian warnings are some nuisance
that's OK to fix later.  There are more real bugs indicated by the
endianess warnings than by any other warnings.

Besides, the endian warnings can be disabled by adding "-Wno-bitwise" to
the sparse flags.  Not that I would recommend it to anyone.

> 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.

Thanks for the statistics!

-- 
Regards,
Pavel Roskin

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

[Johannes Berg]

[-- 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 --]

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

[Harvey Harrison]

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

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

[Sam Ravnborg]

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

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

[Johannes Berg]

[-- 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 --]

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

[Harvey Harrison]

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

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

[Johannes Berg]

[-- 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 --]

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

[Harvey Harrison]

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

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

[Al Viro]

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.

__bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]

[Sam Ravnborg]

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
> 

Re: __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]

[Al Viro]

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.

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

[Johannes Berg]

[-- 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 --]