Re: [RFC][PATCH 00/10] Sparse: Git's "make check" target

From: Josh Triplett <josh@freedesktop.org>
Date: Jun 13, 2007, 20:25 UTC
Message-ID: <467052A4.6080706@freedesktop.org>
In-Reply-To: <466ED9CE.3000800@ramsay1.demon.co.uk>

Ramsay Jones wrote:

Show 15 quoted lines

> Josh Triplett wrote:
>> Ramsay Jones wrote:
>>> fix most of those problems. (the output from "make check" was about 16k
>>> lines at one point!). Git also tickled a bug in sparse 0.2, which resulted
>>> in some 120+ lines of bogus warnings; that was fixed in version 0.3 (commit
>>> 0.2-15-gef25961).  As a result, sparse version 0.3 + my patches, elicits 106
>>> lines of output from "make check".
>> One note about using Sparse with Git: you almost certainly don't want to pass
>> -Wall to sparse, and current Git passes CFLAGS to Sparse which will do exactly
>> that.  -Wall turns on all possible Sparse warnings, including nitpicky
>> warnings and warnings with a high false positive rate.
> 
> I have to say that, my initial reaction, was to disagree; I certainly want to
> pass -Wall to sparse! Why not? Did you have any particular warnings in mind?
> (I haven't noticed any that were nitpicky or had a high false positive rate!)

If you don't mind the set of warnings you get, then sure, use -Wall.

Some of the ones I had in mind:
* -Wshadow.  Not everyone cares.
* -Wptr-subtraction-blows.  This warns any time you do ptr2 - ptr1.
* -Wundefined-preprocessor.  This warns if you ever do
  #if SYMBOL
  when SYMBOL might not actually have a definition.  Many projects do exactly
  that, and the C standard allows it.
* -Wtypesign.  Off by default for the same reason that GCC doesn't give sign
   mismatches by default: too many codebases with too many sloppy signedness
   issues that drown out other issues.

Show 5 quoted lines

> ...  You should start from
>> the default set of Sparse warnings, and add additional warnings as desired, or
>> turn off those you absolutely can't live with.  
> 
> Why not "-Wall -Wno-nitpicky -Wno-false-positive" ;-)

If you don't mind that, then sure. You might have to adjust the warning list to taste from time to time. But please do use -Wall if you feel comfortable with the warnings it produces.

Show 9 quoted lines

> ... Current Sparse from Git (post
>> 0.3, after commit e18c1014449adf42520daa9d3e53f78a3d98da34) has a change to
>> cgcc to filter out -Wall, so you can pass -Wall to GCC but not Sparse.  
> 
> Yes, I noticed that. Again, I'm not sure I agree.
> I didn't comment on that patch, because my exposure to sparse is very limited.
> So far I've only run it on git, so I can hardly claim any great experience with
> the output from sparse. However, 105 lines of output (which represents 71 warnings)
> for 72,974 lines of C (in 179 .c files) did not seem at all unreasonable.

True; for a project the size of Git, you can reasonably handle all the warnings as you did.

If you want to use -Wall with sparse, you can always pass -Wall to sparse directly, or use CHECK="sparse -Wall" cgcc.

- Josh Triplett