archive mirror
 help / color / mirror / Atom feed
From: Jeff King <>
To: Junio C Hamano <>
Cc: "Ævar Arnfjörð Bjarmason" <>,
	"Derrick Stolee" <>,
	"Derrick Stolee via GitGitGadget" <>,
Subject: Re: [PATCH] ci: update 'static-analysis' to Ubuntu 22.04
Date: Sat, 27 Aug 2022 08:58:41 -0400	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <xmqqczcnymtd.fsf@gitster.g>

On Fri, Aug 26, 2022 at 09:46:54AM -0700, Junio C Hamano wrote:

> > Yeah, my general skepticism and disappointment above notwithstanding,
> > this seems like the best path forward from here. I tried a few other
> > tricks (like --macro-file and --iso-file), but if its parser chokes, I
> > don't think there's much we can do about it. Even if we wrote a patch to
> > coccinelle itself (and I have no interest in doing that myself), it
> > would take a while to become available.
> If it is just a single unused.cocci, I would actually think removing
> it would be a much better path forward.  UNUSED() that renames to
> help folks without checking compilers would help noticing bad code
> much earlier than unused.cocci many contributors are not running
> themselves anyway.

I doubt that it is just unused.cocci. If I understand correctly, the new
syntax is choking coccinelle's parser, so it is missing segments of the
code from its analysis.

That shows up as a false positive for unused.cocci, because its patch is
something like "find me spots that have X followed by Y, with no Z in
between (and then delete X and Y)". And the breakage is that it doesn't
notice Z, so it thinks it has found such a spot.

But other rules are of the form "find me X (and turn it into Y)". If the
tool fails to parse code that contains X, then we'll get a false
negative: it won't generate output when it should. But we don't notice
any of those, because there are no true positives in the code base right
now. But it would presumably fail to find some of them if there were.
And more importantly, it makes the tool just as useless as false

If it were a matter of choosing one static analysis over another (unused
parameters versus coccinelle), we might have something tricky to decide.
But Ævar's non-parenthesized suggestion is a practical workaround. I
don't like it as much as what I posted originally, but it's only a
little less ergonomic, and lets us keep using coccinelle (which, despite
the headaches I've had tangling with it over the years, has produced
useful outcomes).


  reply	other threads:[~2022-08-27 12:58 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-23 17:28 [PATCH] ci: update 'static-analysis' to Ubuntu 22.04 Derrick Stolee via GitGitGadget
2022-08-24 14:40 ` Johannes Schindelin
2022-08-24 19:59   ` Junio C Hamano
2022-08-24 23:43 ` Junio C Hamano
2022-08-25  0:30   ` Derrick Stolee
2022-08-25  4:43     ` Junio C Hamano
2022-08-25 10:47       ` Ævar Arnfjörð Bjarmason
2022-08-25 16:08         ` Junio C Hamano
2022-08-25 17:09           ` [PATCH 0/2] git-compat-util.h: change UNUSED(var) to UNUSED Ævar Arnfjörð Bjarmason
2022-08-25 17:09             ` [PATCH 1/2] git-compat-util.h: use "UNUSED", not "UNUSED(var)" Ævar Arnfjörð Bjarmason
2022-08-25 17:09             ` [PATCH 2/2] git-compat-util.h: use "deprecated" for UNUSED variables Ævar Arnfjörð Bjarmason
2022-08-26  7:52             ` [PATCH 0/2] git-compat-util.h: change UNUSED(var) to UNUSED Jeff King
2022-08-26  7:48         ` [PATCH] ci: update 'static-analysis' to Ubuntu 22.04 Jeff King
2022-08-26 16:46           ` Junio C Hamano
2022-08-27 12:58             ` Jeff King [this message]
2022-08-29  5:56               ` Junio C Hamano
2022-08-29 10:29             ` Ævar Arnfjörð Bjarmason
2022-08-31 15:12               ` Jeff King
2022-08-31  8:44             ` SZEDER Gábor
2022-08-31 12:13               ` Ævar Arnfjörð Bjarmason
2022-08-31 15:24                 ` Jeff King
2022-08-31 19:19                   ` Junio C Hamano
2022-08-31 18:05                 ` SZEDER Gábor
2022-08-31 19:29                   ` Ævar Arnfjörð Bjarmason
2022-08-25 14:57 ` Ævar Arnfjörð Bjarmason
2022-08-25 16:17   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).