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

On Wed, Aug 24 2022, Junio C Hamano wrote:

> Derrick Stolee <> writes:
>>> We probably need to fix or revert/remove rules we have in
>>> unused.cocci that makes bogus "suggestion".
>> Yes, this is definitely a bogus suggestion. It's probable that it
>> is picked up by the newer version of Coccinelle.
> Yes, I think we should tentatively disable the offending one until
> we know how to properly "fix" it.

I'm seeing this relatively late, it would be nice to get a CC on
discussions of of one's code :)

This is indeed completely broken, but it's not the unused.cocci rule
that's broken.

What's happening here is that coccinelle can no longer properly parse
the file after the UNUSED() macros were applied to refs.c.

Try running with "--verbose-match --verbose-parsing", on
"seen". Deleting the UNUSED() from warn_if_dangling_symref() happens to
"fix" it, but it's only working as a result of some hack. Coccinelle is
running into some unbalanced paren issue, and it happens to balance out
with that.

I don't think there's any issue here in unused.cocci, it just happens to
be the rule that's unlucky enough to fire like that in the face of these
parse errors.

We should probably coerce coccinelle into stopping in general if it's
encountering "BAD:!!!!!" parse errors behind the scenes, as it clearly
results in broken logic, but offhand (and from briefly browsing the
manpage) I don't know a way to do that.

But the fix here isn't to delete unused.cocci, but to hold off on the
UNUSEwork D() patches until we figure out how to make coccinelle jive with

One thing that *would* fix it is to go with the approach I suggested in,
i.e. to not use an "UNUSED(var)" form, but just "UNUSED".

I tried that just now with this hack, which wouldn't even compile with
the compiler, but coccinelle is seemingly smart enough to ignore unknown
tokens it doesn't know about if they're not introducing parens (i.e. I
didn't even have to define UNUSED2).

It's also not that it punted out entirely, with this changing
refs_verify_refname_available() so that "referent" actually isn't unused
for real would have unused.cocci suggest the same removal, so we're
managing to fully apply rules to the file:
	diff --git a/refs.c b/refs.c
	index 607694c2662..37e7d88920c 100644
	--- a/refs.c
	+++ b/refs.c
	@@ -442,7 +442,7 @@ struct warn_if_dangling_data {
	 static int warn_if_dangling_symref(const char *refname,
	-				   const struct object_id *UNUSED(oid),
	+				   const struct object_id *oid UNUSED2,
	 				   int flags, void *cb_data)
	 	struct warn_if_dangling_data *d = cb_data;
	@@ -982,7 +982,7 @@ static void set_read_ref_cutoffs(struct read_ref_at_cb *cb,
	 static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
	-			   const char *UNUSED(email),
	+			   const char *email UNUSED2,
	 			   timestamp_t timestamp, int tz,
	 			   const char *message, void *cb_data)
	@@ -1024,9 +1024,9 @@ static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
	 	return cb->found_it;
	-static int read_ref_at_ent_newest(struct object_id *UNUSED(ooid),
	+static int read_ref_at_ent_newest(struct object_id *ooid UNUSED2,
	 				  struct object_id *noid,
	-				  const char *UNUSED(email),
	+				  const char *email UNUSED2,
	 				  timestamp_t timestamp, int tz,
	 				  const char *message, void *cb_data)
	@@ -1039,7 +1039,7 @@ static int read_ref_at_ent_newest(struct object_id *UNUSED(ooid),
	 static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
	-				  const char *UNUSED(email),
	+				  const char *email UNUSED2,
	 				  timestamp_t timestamp, int tz,
	 				  const char *message, void *cb_data)
	@@ -1904,7 +1904,7 @@ struct ref_store_hash_entry
	 	char name[FLEX_ARRAY];
	-static int ref_store_hash_cmp(const void *UNUSED(cmp_data),
	+static int ref_store_hash_cmp(const void *cmp_data UNUSED2,
	 			      const struct hashmap_entry *eptr,
	 			      const struct hashmap_entry *entry_or_key,
	 			      const void *keydata)

  reply	other threads:[~2022-08-25 10:59 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 [this message]
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
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).