Re: [RFC/PATCH] Makefile: suppress some cppcheck false-positives

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 16, 2016, 18:43 UTC
Message-ID: <xmqqshpnsoij.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161215232240.19427-1-judge.packham@gmail.com>

Chris Packham <judge.packham@gmail.com> writes:

Show 7 quoted lines

> -CPPCHECK_FLAGS = --force --quiet --inline-suppr $(if $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD))
> +CPPCHECK_SUPP = --suppressions-list=nedmalloc.supp \
> +	--suppressions-list=regcomp.supp
> +
> +CPPCHECK_FLAGS = --force --quiet --inline-suppr \
> +	$(if $(CPPCHECK_ADD),--enable=$(CPPCHECK_ADD)) \
> +	$(CPPCHECK_SUPP)

Has it been agreed that it is a good idea to tie $(CPPCHECK_ADD) only to --enable? I somehow thought I saw an objection to this.

Show 24 quoted lines

> diff --git a/nedmalloc.supp b/nedmalloc.supp
> new file mode 100644
> index 000000000..37bd54def
> --- /dev/null
> +++ b/nedmalloc.supp
> @@ -0,0 +1,4 @@
> +nullPointer:compat/nedmalloc/malloc.c.h:4093
> +nullPointer:compat/nedmalloc/malloc.c.h:4106
> +memleak:compat/nedmalloc/malloc.c.h:4646
> +
> diff --git a/regcomp.supp b/regcomp.supp
> new file mode 100644
> index 000000000..3ae023c26
> --- /dev/null
> +++ b/regcomp.supp
> @@ -0,0 +1,8 @@
> +memleak:compat/regex/regcomp.c:3086
> +memleak:compat/regex/regcomp.c:3634
> +memleak:compat/regex/regcomp.c:3086
> +memleak:compat/regex/regcomp.c:3634
> +uninitvar:compat/regex/regcomp.c:2802
> +uninitvar:compat/regex/regcomp.c:2805
> +memleak:compat/regex/regcomp.c:532
> +

Yuck for both files for multiple reasons.

I do not think it is a good idea to allow these files to clutter the top-level of tree. How often do we expect that we may have to add more of these files? Every time we start borrowing code from third parties?

What is the goal we want to achieve by running cppcheck?

 a. Our code must be clean but we do not bother "fixing" [*1*] the
    code we borrow from third parties and squelch output instead?

 b. Both our own code and third party code we borrow need to be free
    of errors and misdetections from cppcheck?

 c. Something else?

If a. is what we aim for, perhaps a better option may be not to run cppcheck for the code we borrowed from third-party at all in the first place.

If b. is our goal, we need to make sure that the false positive rate of cppcheck is acceptably low.

[Footnote]

*1* "Fixing" a real problem it uncovers is a good thing, but it may
    have to also involve working around false positives reported by
    cppcheck, either by rewriting a perfectly correct code to please
    it, or adding these line-number based suppression that is
    totally unworkable.  Like Peff, I'm worried that we will end up
    with two static checkers fighting each other, and no good way to
    please both.