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

[Josh Triplett <josh@freedesktop.org>]

[-- Attachment #1: Type: text/plain, Size: 2811 bytes --]

Ramsay Jones wrote:
> 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.

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]