On Wed, Dec 14, 2016 at 1:28 AM, Jeff King <peff@xxxxxxxx> wrote: > On Tue, Dec 13, 2016 at 07:15:10AM -0500, Jeff King wrote: > >> I think these last two are a good sign that we need to be feeding the >> list of source files to cppcheck. I tried your patch and it also started >> looking in t/perf/build, which are old versions of git built to serve >> the performance-testing suite. >> >> See the way that the "tags" target is handled for a possible approach. > > Maybe something like this: > > diff --git a/Makefile b/Makefile > index 8b5976d88..e7684ae63 100644 > --- a/Makefile > +++ b/Makefile > @@ -2638,4 +2638,6 @@ cover_db_html: cover_db > .PHONY: cppcheck > > cppcheck: > - cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) . > + $(FIND_SOURCE_FILES) |\ > + grep -v ^t/t |\ > + xargs cppcheck --force --quiet --inline-suppr $(CPPCHECK_ADD) Will look at something like this for v2. > >> My main complaint with any static checker is how we can handle false >> positives. [...] <snip> > So I think it is capable of finding real problems, but I think we'd need > some way of squelching false positives, preferably in a way that carries > forward as the code changes (so not just saying "foo.c:1234 is a false > positive", which will break when it becomes "foo.c:1235"). If we're prepared to wear them, the --inline-suppr will let us annotate the code to avoid the false-positives. Suppressions can also be specified with --suppressions-list=file-with-suppressions but that would suffer from the moving target problem although you can specify the file without the line number to squash a class of warning for a whole file.