* 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
* 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
* __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 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