Home » Mailing list archives » linux-kernel » 2008 » February » 21

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

Previous thread: MMC card detection by Haavard Skinnemoen on Thursday, February 21, 2008 - 2:46 pm. (3 messages)

Next thread: [announce] Xtables, Xtables-addons 1.5.1 and Writing Xtables Modules by Jan Engelhardt on Thursday, February 21, 2008 - 3:37 pm. (3 messages)
[view original message]
From: Johannes Berg <johannes@...>
To: Sam Ravnborg <sam@...>
Cc: John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 8:42 am

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=20

#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


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
[view original message]
From: Al Viro <viro@...>
To: Johannes Berg <johannes@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 5:16 pm

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.

--
[view original message]
From: Johannes Berg <johannes@...>
To: Al Viro <viro@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Friday, February 22, 2008 - 10:02 am

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
[view original message]
From: Sam Ravnborg <sam@...>
To: Johannes Berg <johannes@...>
Cc: John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:06 pm

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

--
[view original message]
From: Johannes Berg <johannes@...>
To: Sam Ravnborg <sam@...>
Cc: John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:09 pm

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
[view original message]
From: Harvey Harrison <harvey.harrison@...>
To: Johannes Berg <johannes@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:25 pm

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...
[ message continues ]
[view original message]
From: Johannes Berg <johannes@...>
To: Harvey Harrison <harvey.harrison@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:29 pm

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
[view original message]
From: Harvey Harrison <harvey.harrison@...>
To: Johannes Berg <johannes@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:33 pm

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


--
[view original message]
From: Harvey Harrison <harvey.harrison@...>
To: Johannes Berg <johannes@...>
Cc: Sam Ravnborg <sam@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: [PATCH] mac80211: check endianness/types in sparse runs
Date: Thursday, February 21, 2008 - 4:01 pm

Potentially expensive pointer subtraction 640:....


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


--
[view original message]
From: Sam Ravnborg <sam@...>
To: Harvey Harrison <harvey.harrison@...>, Al Viro <viro@...>
Cc: Johannes Berg <johannes@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Al Viro <viro@...>, Linux Kernel list <linux-kernel@...>
Subject: __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]
Date: Thursday, February 21, 2008 - 6:32 pm

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


--
[view original message]
From: Al Viro <viro@...>
To: Sam Ravnborg <sam@...>
Cc: Harvey Harrison <harvey.harrison@...>, Al Viro <viro@...>, Johannes Berg <johannes@...>, John Linville <linville@...>, linux-wireless <linux-wireless@...>, Stefano Brivio <stefano.brivio@...>, Linux Kernel list <linux-kernel@...>
Subject: Re: __bitwise versus __bitwise__ [Was: [PATCH] mac80211: check endianness/types in sparse runs]
Date: Thursday, February 21, 2008 - 6:38 pm

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

--
Previous thread: MMC card detection by Haavard Skinnemoen on Thursday, February 21, 2008 - 2:46 pm. (3 messages)

Next thread: [announce] Xtables, Xtables-addons 1.5.1 and Writing Xtables Modules by Jan Engelhardt on Thursday, February 21, 2008 - 3:37 pm. (3 messages)