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

linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] mac80211: check endianness/types in sparse runs
@ 2008-02-20 10:38 Johannes Berg
  2008-02-20 19:59 ` Sam Ravnborg
  0 siblings, 1 reply; 14+ messages in thread
From: Johannes Berg @ 2008-02-20 10:38 UTC (permalink / raw)
  To: John Linville; +Cc: linux-wireless, Stefano Brivio, Sam Ravnborg

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__



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

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-20 10:38 [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
@ 2008-02-20 19:59 ` Sam Ravnborg
  2008-02-20 20:07   ` Pavel Roskin
  2008-02-21 12:42   ` Johannes Berg
  0 siblings, 2 replies; 14+ messages in thread
From: Sam Ravnborg @ 2008-02-20 19:59 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, linux-wireless, Stefano Brivio

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

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

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-20 19:59 ` Sam Ravnborg
@ 2008-02-20 20:07   ` Pavel Roskin
  2008-02-21 12:42   ` Johannes Berg
  1 sibling, 0 replies; 14+ messages in thread
From: Pavel Roskin @ 2008-02-20 20:07 UTC (permalink / raw)
  To: Sam Ravnborg; +Cc: Johannes Berg, John Linville, linux-wireless, Stefano Brivio

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

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

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-20 19:59 ` Sam Ravnborg
  2008-02-20 20:07   ` Pavel Roskin
@ 2008-02-21 12:42   ` Johannes Berg
  2008-02-21 20:01     ` Harvey Harrison
                       ` (2 more replies)
  1 sibling, 3 replies; 14+ 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] 14+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ 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; 14+ 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] 14+ messages in thread

* Re: [PATCH] mac80211: check endianness/types in sparse runs
  2008-02-21 12:42   ` 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; 14+ 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] 14+ 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; 14+ 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] 14+ messages in thread

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

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-20 10:38 [PATCH] mac80211: check endianness/types in sparse runs Johannes Berg
2008-02-20 19:59 ` Sam Ravnborg
2008-02-20 20:07   ` Pavel Roskin
2008-02-21 12:42   ` 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;
as well as URLs for NNTP newsgroup(s).