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

Sam Ravnborg
2008-02-21T20:06:22Z

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