git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/11] annotating unused function parameters
@ 2022-08-19 10:07 Jeff King
  2022-08-19 10:08 ` [PATCH 01/11] git-compat-util: add UNUSED macro Jeff King
                   ` (11 more replies)
  0 siblings, 12 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:07 UTC (permalink / raw)
  To: git

I've been carrying a bunch of patches (for almost 4 years now!) that get
the code base compiling cleanly with -Wunused-parameter. This is a
useful warning in my opinion; it found real bugs[1] when applied to the
whole code base. So it would be nice to be able to turn it on all the
time and get the same protection going forward.

Unfortunately, there's a catch: some functions really do need to have
unused parameters, because they're conforming to a specific interface.
Some reasons might be:

  - they're passed as callback function pointers

  - they're virtual functions called as pointers via a struct

  - they're compat functions that implement a well-known interface (this
    is especially true for "trivial" wrappers that are noops, or return
    ENOSYS, etc)

So we need some way to annotate the acceptable cases. This series
introduces a macro to do so. That's in the first patch. The other ten
apply it in various situations, with the end goal being that we could
flip the warning on for DEVELOPER=1 builds. This series doesn't get us
all the way there! I have ~100 more patches adding similar annotations.
My goal here is to give a taste of the direction and make sure it's
where we want to go.

I've grouped related annotations into a single patch. They should be
fairly easy to review. The annotation macro is written in such a way
that the compiler will complain if it's wrong. So really what you care
about in each is:

  - is this a case where we really want to be annotating and not
    removing unused parameters (or using them)? Each commit should
    explain this, and it's usually obvious (e.g., a specific type of
    callback)

  - does each hunk in the patch match the group

  - did I introduce any formatting problems along the way :)

  - (optional) are there other cases that should be in the group. This
    is probably not worth your time to go digging for. The other ~100
    patches aren't carefully organized and written up yet, so it's quite
    possible that I've mistakenly left a related case there that _could_
    be folded in. But those shake out much easier as the number of cases
    is reduced, and you can ask -Wunused-parameter to find them for you.

And of course the most important question is: do we like this direction
overall. This mass-annotation is a one-time pain. Going forward, the
only work would be requiring people to annotate new functions they add
(which again, is mostly going to be callbacks). IMHO it's worth it. In
addition to possibly finding errors, I think the annotations serve as an
extra clue for people reading the code about what the author intended.

So without further ado, the patches are:

  [01/11]: git-compat-util: add UNUSED macro
  [02/11]: refs: mark unused each_ref_fn parameters
  [03/11]: refs: mark unused reflog callback parameters
  [04/11]: refs: mark unused virtual method parameters
  [05/11]: transport: mark bundle transport_options as unused
  [06/11]: streaming: mark unused virtual method parameters
  [07/11]: config: mark unused callback parameters
  [08/11]: hashmap: mark unused callback parameters
  [09/11]: mark unused read_tree_recursive() callback parameters
  [10/11]: run-command: mark unused async callback parameters
  [11/11]: is_path_owned_by_current_uid(): mark "report" parameter as unused

 add-interactive.c           |  2 +-
 archive-tar.c               |  5 +++--
 archive-zip.c               |  5 +++--
 archive.c                   |  3 ++-
 attr.c                      |  4 ++--
 bisect.c                    |  7 ++++---
 bloom.c                     |  4 ++--
 builtin/am.c                |  2 +-
 builtin/bisect--helper.c    | 12 +++++++-----
 builtin/checkout.c          |  4 ++--
 builtin/commit-graph.c      |  2 +-
 builtin/config.c            |  8 +++++---
 builtin/describe.c          |  5 +++--
 builtin/difftool.c          | 10 +++++-----
 builtin/fast-export.c       |  2 +-
 builtin/fast-import.c       |  2 +-
 builtin/fetch.c             |  9 +++++----
 builtin/fsck.c              | 12 +++++++-----
 builtin/gc.c                |  5 +++--
 builtin/log.c               |  7 ++++---
 builtin/ls-tree.c           | 13 ++++++++-----
 builtin/multi-pack-index.c  |  2 +-
 builtin/name-rev.c          |  3 ++-
 builtin/pack-objects.c      | 12 +++++++-----
 builtin/receive-pack.c      |  4 ++--
 builtin/reflog.c            |  3 ++-
 builtin/remote.c            | 14 +++++++++-----
 builtin/repack.c            |  4 ++--
 builtin/rev-parse.c         |  6 ++++--
 builtin/show-branch.c       |  6 +++---
 builtin/show-ref.c          |  7 ++++---
 builtin/stash.c             |  9 ++++++---
 builtin/submodule--helper.c |  5 +++--
 color.c                     |  2 +-
 commit-graph.c              |  4 ++--
 commit.c                    |  5 +++--
 compat/terminal.c           |  2 +-
 config.c                    |  7 ++++---
 convert.c                   |  4 ++--
 delta-islands.c             |  4 ++--
 diff.c                      |  5 +++--
 dir.c                       |  4 ++--
 environment.c               |  4 ++--
 fetch-pack.c                | 14 +++++++++-----
 git-compat-util.h           | 13 +++++++++++--
 gpg-interface.c             |  2 +-
 hashmap.c                   | 10 +++++-----
 help.c                      |  5 +++--
 http-backend.c              |  2 +-
 ident.c                     |  2 +-
 ll-merge.c                  |  3 ++-
 log-tree.c                  |  3 ++-
 ls-refs.c                   |  3 ++-
 merge-recursive.c           | 12 ++++++------
 name-hash.c                 |  4 ++--
 negotiator/default.c        |  3 ++-
 negotiator/skipping.c       |  3 ++-
 notes.c                     |  5 +++--
 object-name.c               | 10 +++++++---
 object-store.h              |  2 +-
 oidmap.c                    |  2 +-
 packfile.c                  |  2 +-
 pager.c                     |  3 ++-
 patch-ids.c                 |  2 +-
 pretty.c                    |  3 ++-
 range-diff.c                |  6 ++++--
 ref-filter.c                |  2 +-
 reflog.c                    | 16 ++++++++++------
 refs.c                      | 23 ++++++++++++++---------
 refs/files-backend.c        | 14 ++++++++------
 refs/iterator.c             |  6 +++---
 refs/packed-backend.c       | 14 ++++++++------
 remote.c                    | 22 +++++++++++++---------
 replace-object.c            |  3 ++-
 revision.c                  | 18 +++++++++++-------
 send-pack.c                 |  2 +-
 sequencer.c                 |  5 +++--
 server-info.c               |  3 ++-
 shallow.c                   | 12 ++++++++----
 streaming.c                 |  6 +++---
 strmap.c                    |  4 ++--
 sub-process.c               |  4 ++--
 submodule-config.c          |  8 ++++----
 submodule.c                 | 13 ++++++++-----
 t/helper/test-config.c      |  2 +-
 t/helper/test-ref-store.c   |  4 ++--
 t/helper/test-userdiff.c    |  2 +-
 trailer.c                   |  6 ++++--
 transport.c                 |  2 +-
 upload-pack.c               |  7 ++++---
 walker.c                    |  6 ++++--
 wt-status.c                 | 14 +++++++++-----
 92 files changed, 337 insertions(+), 234 deletions(-)

-Peff

[1] Here are some examples of bugs found by -Wunused-parameter:

     - https://lore.kernel.org/git/20181102063156.GA30252@sigill.intra.peff.net/
     - https://lore.kernel.org/git/20181102052239.GA19162@sigill.intra.peff.net/


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 01/11] git-compat-util: add UNUSED macro
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 02/11] refs: mark unused each_ref_fn parameters Jeff King
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

In preparation for compiling with -Wunused-parameter, we'd like to be
able to annotate some function parameters as false positives (e.g.,
parameters which must exist to conform to a callback interface).

Ideally our annotation will:

  - be portable, turning into nothing on platforms which don't support
    it

  - be easy to read, without looking too syntactically odd or taking
    attention away from the rest of the parameters

  - help us notice when a parameter marked as unused is actually used,
    which keeps our annotations accurate. In theory a compiler could
    tell us this easily, but gcc has no such warning. Clang has
    -Wused-but-marked-unused, but it triggers false positives with our
    MAYBE_UNUSED annotation (e.g., for commit-slab functions)

This patch introduces an UNUSED() macro which takes the parameter name
as an argument. That lets us tweak the name in such a way that we'll
notice if somebody tries to use it. It looks like this in use:

  int some_ref_cb(const char *refname,
                  const struct object_id *UNUSED(oid),
                  int UNUSED(flags),
                  void *UNUSED(data))
  {
        printf("got refname %s", refname);
        return 0;
  }

Because the unused parameter names are rewritten behind the scenes to
UNUSED_oid, etc, adding code like:

  printf("oid is %s", oid_to_hex(oid));

will fail compilation with "oid undeclared". Sadly, the "did you mean"
feature of modern compilers is not generally smart enough to suggest the
"unused" name. If we used a very short prefix like U_oid, that does
convince gcc to say "did you mean", but since the "U_" in the suggestion
isn't much of a hint, it doesn't really help. In practice, a look at the
function definition usually makes the problem pretty obvious.

Note that we have to put the definition of UNUSED early in
git-compat-util.h, because it will eventually be used for some compat
functions themselves (both directly here and in mingw.h).

Signed-off-by: Jeff King <peff@peff.net>
---
 git-compat-util.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/git-compat-util.h b/git-compat-util.h
index 36a25ae252..c6669db07d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -189,6 +189,12 @@ struct strbuf;
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
+#if defined(__GNUC__)
+#define UNUSED(var) UNUSED_##var __attribute__((unused))
+#else
+#define UNUSED(var) UNUSED_##var
+#endif
+
 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
 # if !defined(_WIN32_WINNT)
 #  define _WIN32_WINNT 0x0600
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 02/11] refs: mark unused each_ref_fn parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
  2022-08-19 10:08 ` [PATCH 01/11] git-compat-util: add UNUSED macro Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 03/11] refs: mark unused reflog callback parameters Jeff King
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

Functions used with for_each_ref(), etc, need to conform to the
each_ref_fn interface. But most of them don't need every parameter;
let's annotate the unused ones to quiet -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 bisect.c                    |  7 ++++---
 builtin/bisect--helper.c    | 12 +++++++-----
 builtin/checkout.c          |  2 +-
 builtin/describe.c          |  3 ++-
 builtin/fetch.c             |  7 ++++---
 builtin/fsck.c              |  7 ++++---
 builtin/gc.c                |  5 +++--
 builtin/name-rev.c          |  3 ++-
 builtin/pack-objects.c      | 12 +++++++-----
 builtin/receive-pack.c      |  2 +-
 builtin/reflog.c            |  3 ++-
 builtin/remote.c            | 12 ++++++++----
 builtin/repack.c            |  4 ++--
 builtin/rev-parse.c         |  6 ++++--
 builtin/show-branch.c       |  6 +++---
 builtin/show-ref.c          |  7 ++++---
 builtin/submodule--helper.c |  5 +++--
 commit-graph.c              |  4 ++--
 delta-islands.c             |  2 +-
 fetch-pack.c                | 12 ++++++++----
 help.c                      |  5 +++--
 http-backend.c              |  2 +-
 log-tree.c                  |  3 ++-
 negotiator/default.c        |  3 ++-
 negotiator/skipping.c       |  3 ++-
 notes.c                     |  5 +++--
 object-name.c               |  3 ++-
 reflog.c                    |  3 ++-
 refs.c                      | 11 +++++++----
 refs/files-backend.c        |  4 +++-
 remote.c                    |  3 ++-
 replace-object.c            |  3 ++-
 revision.c                  |  7 ++++---
 server-info.c               |  3 ++-
 shallow.c                   | 12 ++++++++----
 submodule.c                 | 10 ++++++----
 t/helper/test-ref-store.c   |  2 +-
 upload-pack.c               |  7 ++++---
 walker.c                    |  6 ++++--
 39 files changed, 132 insertions(+), 84 deletions(-)

diff --git a/bisect.c b/bisect.c
index 38b3891f3a..07ccd1bce6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -441,7 +441,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 }
 
 static int register_ref(const char *refname, const struct object_id *oid,
-			int flags, void *cb_data)
+			int UNUSED(flags), void *UNUSED(cb_data))
 {
 	struct strbuf good_prefix = STRBUF_INIT;
 	strbuf_addstr(&good_prefix, term_good);
@@ -1160,8 +1160,9 @@ int estimate_bisect_steps(int all)
 	return (e < 3 * x) ? n : n - 1;
 }
 
-static int mark_for_removal(const char *refname, const struct object_id *oid,
-			    int flag, void *cb_data)
+static int mark_for_removal(const char *refname,
+			    const struct object_id *UNUSED(oid),
+			    int UNUSED(flag), void *cb_data)
 {
 	struct string_list *refs = cb_data;
 	char *ref = xstrfmt("refs/bisect%s", refname);
diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
index 8a052c7111..87c8b2d818 100644
--- a/builtin/bisect--helper.c
+++ b/builtin/bisect--helper.c
@@ -329,8 +329,9 @@ static int check_and_set_terms(struct bisect_terms *terms, const char *cmd)
 	return 0;
 }
 
-static int inc_nr(const char *refname, const struct object_id *oid,
-		  int flag, void *cb_data)
+static int inc_nr(const char *UNUSED(refname),
+		  const struct object_id *UNUSED(oid),
+		  int UNUSED(flag), void *cb_data)
 {
 	unsigned int *nr = (unsigned int *)cb_data;
 	(*nr)++;
@@ -518,7 +519,7 @@ static int bisect_append_log_quoted(const char **argv)
 }
 
 static int add_bisect_ref(const char *refname, const struct object_id *oid,
-			  int flags, void *cb)
+			  int UNUSED(flags), void *cb)
 {
 	struct add_bisect_ref_data *data = cb;
 
@@ -1134,8 +1135,9 @@ static int bisect_visualize(struct bisect_terms *terms, const char **argv, int a
 	return res;
 }
 
-static int get_first_good(const char *refname, const struct object_id *oid,
-			  int flag, void *cb_data)
+static int get_first_good(const char *UNUSED(refname),
+			  const struct object_id *oid,
+			  int UNUSED(flag), void *cb_data)
 {
 	oidcpy(cb_data, oid);
 	return 1;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f9d63d80b9..713410ce2c 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -990,7 +990,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 
 static int add_pending_uninteresting_ref(const char *refname,
 					 const struct object_id *oid,
-					 int flags, void *cb_data)
+					 int UNUSED(flags), void *cb_data)
 {
 	add_pending_oid(cb_data, refname, oid, UNINTERESTING);
 	return 0;
diff --git a/builtin/describe.c b/builtin/describe.c
index a76f1a1a7a..3af36483f2 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -140,7 +140,8 @@ static void add_to_known_names(const char *path,
 	}
 }
 
-static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)
+static int get_name(const char *path, const struct object_id *oid,
+		    int UNUSED(flag), void *UNUSED(cb_data))
 {
 	int is_tag = 0;
 	struct object_id peeled;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc5cecb483..9e7c8099fe 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -329,7 +329,7 @@ static struct refname_hash_entry *refname_hash_add(struct hashmap *map,
 
 static int add_one_refname(const char *refname,
 			   const struct object_id *oid,
-			   int flag, void *cbdata)
+			   int UNUSED(flag), void *cbdata)
 {
 	struct hashmap *refname_map = cbdata;
 
@@ -1462,8 +1462,9 @@ static void set_option(struct transport *transport, const char *name, const char
 }
 
 
-static int add_oid(const char *refname, const struct object_id *oid, int flags,
-		   void *cb_data)
+static int add_oid(const char *UNUSED(refname),
+		   const struct object_id *oid,
+		   int UNUSED(flags), void *cb_data)
 {
 	struct oid_array *oids = cb_data;
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 6c73092f10..36f1524614 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -502,8 +502,9 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
 	return 0;
 }
 
-static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
-			      int flag, void *cb_data)
+static int fsck_handle_reflog(const char *logname,
+			      const struct object_id *UNUSED(oid),
+			      int UNUSED(flag), void *cb_data)
 {
 	struct strbuf refname = STRBUF_INIT;
 
@@ -514,7 +515,7 @@ static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
 }
 
 static int fsck_handle_ref(const char *refname, const struct object_id *oid,
-			   int flag, void *cb_data)
+			   int UNUSED(flag), void *UNUSED(cb_data))
 {
 	struct object *obj;
 
diff --git a/builtin/gc.c b/builtin/gc.c
index eeff2b760e..e4ede128c9 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -782,8 +782,9 @@ struct cg_auto_data {
 	int limit;
 };
 
-static int dfs_on_ref(const char *refname,
-		      const struct object_id *oid, int flags,
+static int dfs_on_ref(const char *UNUSED(refname),
+		      const struct object_id *oid,
+		      int UNUSED(flags),
 		      void *cb_data)
 {
 	struct cg_auto_data *data = (struct cg_auto_data *)cb_data;
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 580b1eb170..c4dc143c4b 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -344,7 +344,8 @@ static int cmp_by_tag_and_age(const void *a_, const void *b_)
 	return a->taggerdate != b->taggerdate;
 }
 
-static int name_ref(const char *path, const struct object_id *oid, int flags, void *cb_data)
+static int name_ref(const char *path, const struct object_id *oid,
+		    int UNUSED(flags), void *cb_data)
 {
 	struct object *o = parse_object(the_repository, oid);
 	struct name_ref_data *data = cb_data;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 39e28cfcaf..eb93e5c8fe 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -759,8 +759,8 @@ static enum write_one_status write_one(struct hashfile *f,
 	return WRITE_ONE_WRITTEN;
 }
 
-static int mark_tagged(const char *path, const struct object_id *oid, int flag,
-		       void *cb_data)
+static int mark_tagged(const char *UNUSED(path), const struct object_id *oid,
+		       int UNUSED(flag), void *UNUSED(cb_data))
 {
 	struct object_id peeled;
 	struct object_entry *entry = packlist_find(&to_pack, oid);
@@ -3035,7 +3035,8 @@ static void add_tag_chain(const struct object_id *oid)
 	}
 }
 
-static int add_ref_tag(const char *tag, const struct object_id *oid, int flag, void *cb_data)
+static int add_ref_tag(const char *UNUSED(tag), const struct object_id *oid,
+		       int UNUSED(flag), void *UNUSED(cb_data))
 {
 	struct object_id peeled;
 
@@ -3950,8 +3951,9 @@ static void record_recent_commit(struct commit *commit, void *data)
 }
 
 static int mark_bitmap_preferred_tip(const char *refname,
-				     const struct object_id *oid, int flags,
-				     void *_data)
+				     const struct object_id *oid,
+				     int UNUSED(flags),
+				     void *UNUSED(data))
 {
 	struct object_id peeled;
 	struct object *object;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 31b48e728b..afd36c9c53 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -291,7 +291,7 @@ static void show_ref(const char *path, const struct object_id *oid)
 }
 
 static int show_ref_cb(const char *path_full, const struct object_id *oid,
-		       int flag, void *data)
+		       int UNUSED(flag), void *data)
 {
 	struct oidset *seen = data;
 	const char *path = strip_namespace(path_full);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 8123956847..ab54d73611 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -56,7 +56,8 @@ struct worktree_reflogs {
 	struct string_list reflogs;
 };
 
-static int collect_reflog(const char *ref, const struct object_id *oid, int unused, void *cb_data)
+static int collect_reflog(const char *ref, const struct object_id *UNUSED(oid),
+			  int UNUSED(flags), void *cb_data)
 {
 	struct worktree_reflogs *cb = cb_data;
 	struct worktree *worktree = cb->worktree;
diff --git a/builtin/remote.c b/builtin/remote.c
index c713463d89..b390360f07 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -264,7 +264,8 @@ static const char *abbrev_ref(const char *name, const char *prefix)
 }
 #define abbrev_branch(name) abbrev_ref((name), "refs/heads/")
 
-static int config_read_branches(const char *key, const char *value, void *cb)
+static int config_read_branches(const char *key, const char *value,
+				void *UNUSED(data))
 {
 	const char *orig_key = key;
 	char *name;
@@ -538,7 +539,8 @@ struct branches_for_remote {
 };
 
 static int add_branch_for_removal(const char *refname,
-	const struct object_id *oid, int flags, void *cb_data)
+				  const struct object_id *UNUSED(oid),
+				  int UNUSED(flags), void *cb_data)
 {
 	struct branches_for_remote *branches = cb_data;
 	struct refspec_item refspec;
@@ -580,7 +582,8 @@ struct rename_info {
 };
 
 static int read_remote_branches(const char *refname,
-	const struct object_id *oid, int flags, void *cb_data)
+				const struct object_id *UNUSED(oid),
+				int UNUSED(flags), void *cb_data)
 {
 	struct rename_info *rename = cb_data;
 	struct strbuf buf = STRBUF_INIT;
@@ -953,7 +956,8 @@ static void free_remote_ref_states(struct ref_states *states)
 }
 
 static int append_ref_to_tracked_list(const char *refname,
-	const struct object_id *oid, int flags, void *cb_data)
+				      const struct object_id *UNUSED(oid),
+				      int flags, void *cb_data)
 {
 	struct ref_states *states = cb_data;
 	struct refspec_item refspec;
diff --git a/builtin/repack.c b/builtin/repack.c
index 482b66f57d..ff952dec48 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -514,9 +514,9 @@ struct midx_snapshot_ref_data {
 	int preferred;
 };
 
-static int midx_snapshot_ref_one(const char *refname,
+static int midx_snapshot_ref_one(const char *UNUSED(refname),
 				 const struct object_id *oid,
-				 int flag, void *_data)
+				 int UNUSED(flag), void *_data)
 {
 	struct midx_snapshot_ref_data *data = _data;
 	struct object_id peeled;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index b259d8990a..3c448d438b 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -195,15 +195,17 @@ static int show_default(void)
 	return 0;
 }
 
-static int show_reference(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int show_reference(const char *refname, const struct object_id *oid,
+			  int UNUSED(flag), void *UNUSED(cb_data))
 {
 	if (ref_excluded(ref_excludes, refname))
 		return 0;
 	show_rev(NORMAL, oid, refname);
 	return 0;
 }
 
-static int anti_reference(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int anti_reference(const char *refname, const struct object_id *oid,
+			  int UNUSED(flag), void *UNUSED(cb_data))
 {
 	show_rev(REVERSED, oid, refname);
 	return 0;
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index 64c649c6a2..3ec011bea4 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -404,7 +404,7 @@ static int append_ref(const char *refname, const struct object_id *oid,
 }
 
 static int append_head_ref(const char *refname, const struct object_id *oid,
-			   int flag, void *cb_data)
+			   int UNUSED(flag), void *UNUSED(cb_data))
 {
 	struct object_id tmp;
 	int ofs = 11;
@@ -419,7 +419,7 @@ static int append_head_ref(const char *refname, const struct object_id *oid,
 }
 
 static int append_remote_ref(const char *refname, const struct object_id *oid,
-			     int flag, void *cb_data)
+			     int UNUSED(flag), void *UNUSED(cb_data))
 {
 	struct object_id tmp;
 	int ofs = 13;
@@ -434,7 +434,7 @@ static int append_remote_ref(const char *refname, const struct object_id *oid,
 }
 
 static int append_tag_ref(const char *refname, const struct object_id *oid,
-			  int flag, void *cb_data)
+			  int UNUSED(flag), void *UNUSED(cb_data))
 {
 	if (!starts_with(refname, "refs/tags/"))
 		return 0;
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 5fa207a044..9746537220 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -47,7 +47,7 @@ static void show_one(const char *refname, const struct object_id *oid)
 }
 
 static int show_ref(const char *refname, const struct object_id *oid,
-		    int flag, void *cbdata)
+		    int UNUSED(flag), void *UNUSED(cbdata))
 {
 	if (show_head && !strcmp(refname, "HEAD"))
 		goto match;
@@ -77,8 +77,9 @@ static int show_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int add_existing(const char *refname, const struct object_id *oid,
-			int flag, void *cbdata)
+static int add_existing(const char *refname,
+			const struct object_id *UNUSED(oid),
+			int UNUSED(flag), void *cbdata)
 {
 	struct string_list *list = (struct string_list *)cbdata;
 	string_list_insert(list, refname);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b63f420ece..e24e721458 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -622,8 +622,9 @@ static void print_status(unsigned int flags, char state, const char *path,
 	printf("\n");
 }
 
-static int handle_submodule_head_ref(const char *refname,
-				     const struct object_id *oid, int flags,
+static int handle_submodule_head_ref(const char *UNUSED(refname),
+				     const struct object_id *oid,
+				     int UNUSED(flags),
 				     void *cb_data)
 {
 	struct object_id *output = cb_data;
diff --git a/commit-graph.c b/commit-graph.c
index f2a36032f8..1ab5c3233f 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1639,9 +1639,9 @@ struct refs_cb_data {
 	struct progress *progress;
 };
 
-static int add_ref_to_set(const char *refname,
+static int add_ref_to_set(const char *UNUSED(refname),
 			  const struct object_id *oid,
-			  int flags, void *cb_data)
+			  int UNUSED(flags), void *cb_data)
 {
 	struct object_id peeled;
 	struct refs_cb_data *data = (struct refs_cb_data *)cb_data;
diff --git a/delta-islands.c b/delta-islands.c
index aa98b2e541..13eb96e0c4 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -365,7 +365,7 @@ static void add_ref_to_island(const char *island_name, const struct object_id *o
 }
 
 static int find_island_for_ref(const char *refname, const struct object_id *oid,
-			       int flags, void *data)
+			       int UNUSED(flags), void *UNUSED(data))
 {
 	/*
 	 * We should advertise 'ARRAY_SIZE(matches) - 2' as the max,
diff --git a/fetch-pack.c b/fetch-pack.c
index d35be4177b..bda9d0f433 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -176,8 +176,10 @@ static int rev_list_insert_ref(struct fetch_negotiator *negotiator,
 	return 0;
 }
 
-static int rev_list_insert_ref_oid(const char *refname, const struct object_id *oid,
-				   int flag, void *cb_data)
+static int rev_list_insert_ref_oid(const char *UNUSED(refname),
+				   const struct object_id *oid,
+				   int UNUSED(flag),
+				   void *cb_data)
 {
 	return rev_list_insert_ref(cb_data, oid);
 }
@@ -580,8 +582,10 @@ static int mark_complete(const struct object_id *oid)
 	return 0;
 }
 
-static int mark_complete_oid(const char *refname, const struct object_id *oid,
-			     int flag, void *cb_data)
+static int mark_complete_oid(const char *UNUSED(refname),
+			     const struct object_id *oid,
+			     int UNUSED(flag),
+			     void *UNUSED(cb_data))
 {
 	return mark_complete(oid);
 }
diff --git a/help.c b/help.c
index 991e33f8a6..c5b5848188 100644
--- a/help.c
+++ b/help.c
@@ -781,8 +781,9 @@ struct similar_ref_cb {
 	struct string_list *similar_refs;
 };
 
-static int append_similar_ref(const char *refname, const struct object_id *oid,
-			      int flags, void *cb_data)
+static int append_similar_ref(const char *refname,
+			      const struct object_id *UNUSED(oid),
+			      int UNUSED(flags), void *cb_data)
 {
 	struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data);
 	char *branch = strrchr(refname, '/') + 1;
diff --git a/http-backend.c b/http-backend.c
index 58b83a9f66..20db0ea620 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -505,7 +505,7 @@ static void run_service(const char **argv, int buffer_input)
 }
 
 static int show_text_ref(const char *name, const struct object_id *oid,
-			 int flag, void *cb_data)
+			 int UNUSED(flag), void *cb_data)
 {
 	const char *name_nons = strip_namespace(name);
 	struct strbuf *buf = cb_data;
diff --git a/log-tree.c b/log-tree.c
index 82d9b5f650..4ca872eef4 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -135,7 +135,8 @@ static int ref_filter_match(const char *refname,
 }
 
 static int add_ref_decoration(const char *refname, const struct object_id *oid,
-			      int flags, void *cb_data)
+			      int UNUSED(flags),
+			      void *cb_data)
 {
 	struct object *obj;
 	enum object_type objtype;
diff --git a/negotiator/default.c b/negotiator/default.c
index 434189ae5d..10f0a46e62 100644
--- a/negotiator/default.c
+++ b/negotiator/default.c
@@ -36,7 +36,8 @@ static void rev_list_push(struct negotiation_state *ns,
 }
 
 static int clear_marks(const char *refname, const struct object_id *oid,
-		       int flag, void *cb_data)
+		       int UNUSED(flag),
+		       void *UNUSED(cb_data))
 {
 	struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0);
 
diff --git a/negotiator/skipping.c b/negotiator/skipping.c
index 1236e79224..f2aa58af92 100644
--- a/negotiator/skipping.c
+++ b/negotiator/skipping.c
@@ -72,7 +72,8 @@ static struct entry *rev_list_push(struct data *data, struct commit *commit, int
 }
 
 static int clear_marks(const char *refname, const struct object_id *oid,
-		       int flag, void *cb_data)
+		       int UNUSED(flag),
+		       void *UNUSED(cb_data))
 {
 	struct object *o = deref_tag(the_repository, parse_object(the_repository, oid), refname, 0);
 
diff --git a/notes.c b/notes.c
index 7452e71cc8..3be98e7085 100644
--- a/notes.c
+++ b/notes.c
@@ -924,8 +924,9 @@ int combine_notes_cat_sort_uniq(struct object_id *cur_oid,
 	return ret;
 }
 
-static int string_list_add_one_ref(const char *refname, const struct object_id *oid,
-				   int flag, void *cb)
+static int string_list_add_one_ref(const char *refname,
+				   const struct object_id *UNUSED(oid),
+				   int UNUSED(flag), void *cb)
 {
 	struct string_list *refs = cb;
 	if (!unsorted_string_list_has_string(refs, refname))
diff --git a/object-name.c b/object-name.c
index 4d2746574c..052644977e 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1306,7 +1306,8 @@ struct handle_one_ref_cb {
 };
 
 static int handle_one_ref(const char *path, const struct object_id *oid,
-			  int flag, void *cb_data)
+			  int UNUSED(flag),
+			  void *cb_data)
 {
 	struct handle_one_ref_cb *cb = cb_data;
 	struct commit_list **list = cb->list;
diff --git a/reflog.c b/reflog.c
index 135a1a6e41..ee8aaa78f5 100644
--- a/reflog.c
+++ b/reflog.c
@@ -294,7 +294,8 @@ int should_expire_reflog_ent_verbose(struct object_id *ooid,
 	return expire;
 }
 
-static int push_tip_to_list(const char *refname, const struct object_id *oid,
+static int push_tip_to_list(const char *UNUSED(refname),
+			    const struct object_id *oid,
 			    int flags, void *cb_data)
 {
 	struct commit_list **list = cb_data;
diff --git a/refs.c b/refs.c
index 90bcb27168..34373e8087 100644
--- a/refs.c
+++ b/refs.c
@@ -358,7 +358,8 @@ struct warn_if_dangling_data {
 	const char *msg_fmt;
 };
 
-static int warn_if_dangling_symref(const char *refname, const struct object_id *oid,
+static int warn_if_dangling_symref(const char *refname,
+				   const struct object_id *UNUSED(oid),
 				   int flags, void *cb_data)
 {
 	struct warn_if_dangling_data *d = cb_data;
@@ -934,9 +935,11 @@ 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 *ooid, struct object_id *noid,
-				  const char *email, timestamp_t timestamp,
-				  int tz, const char *message, void *cb_data)
+static int read_ref_at_ent_newest(struct object_id *UNUSED(ooid),
+				  struct object_id *noid,
+				  const char *UNUSED(email),
+				  timestamp_t timestamp, int tz,
+				  const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8db7882aac..972701ce00 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2995,7 +2995,9 @@ static int files_transaction_abort(struct ref_store *ref_store,
 }
 
 static int ref_present(const char *refname,
-		       const struct object_id *oid, int flags, void *cb_data)
+		       const struct object_id *UNUSED(oid),
+		       int UNUSED(flags),
+		       void *cb_data)
 {
 	struct string_list *affected_refnames = cb_data;
 
diff --git a/remote.c b/remote.c
index 618ad5a0f1..723aa8841c 100644
--- a/remote.c
+++ b/remote.c
@@ -2320,7 +2320,8 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb,
 }
 
 static int one_local_ref(const char *refname, const struct object_id *oid,
-			 int flag, void *cb_data)
+			 int UNUSED(flag),
+			 void *cb_data)
 {
 	struct ref ***local_tail = cb_data;
 	struct ref *ref;
diff --git a/replace-object.c b/replace-object.c
index 7bd9aba6ee..17810e5a3a 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -9,7 +9,8 @@
 static int register_replace_ref(struct repository *r,
 				const char *refname,
 				const struct object_id *oid,
-				int flag, void *cb_data)
+				int UNUSED(flag),
+				void *UNUSED(cb_data))
 {
 	/* Get sha1 from refname */
 	const char *slash = strrchr(refname, '/');
diff --git a/revision.c b/revision.c
index f4eee11cc8..23c2bba0d8 100644
--- a/revision.c
+++ b/revision.c
@@ -1543,7 +1543,8 @@ int ref_excluded(struct string_list *ref_excludes, const char *path)
 }
 
 static int handle_one_ref(const char *path, const struct object_id *oid,
-			  int flag, void *cb_data)
+			  int UNUSED(flag),
+			  void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	struct object *object;
@@ -1627,8 +1628,8 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 }
 
 static int handle_one_reflog(const char *refname_in_wt,
-			     const struct object_id *oid,
-			     int flag, void *cb_data)
+			     const struct object_id *UNUSED(oid),
+			     int UNUSED(flag), void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	struct strbuf refname = STRBUF_INIT;
diff --git a/server-info.c b/server-info.c
index 7701d7c20a..d99d9d5f61 100644
--- a/server-info.c
+++ b/server-info.c
@@ -147,7 +147,8 @@ static int update_info_file(char *path,
 }
 
 static int add_info_ref(const char *path, const struct object_id *oid,
-			int flag, void *cb_data)
+			int UNUSED(flag),
+			void *cb_data)
 {
 	struct update_info_ctx *uic = cb_data;
 	struct object *o = parse_object(the_repository, oid);
diff --git a/shallow.c b/shallow.c
index 8cb768ee5f..71ab04f935 100644
--- a/shallow.c
+++ b/shallow.c
@@ -604,8 +604,10 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,
 	free(tmp);
 }
 
-static int mark_uninteresting(const char *refname, const struct object_id *oid,
-			      int flags, void *cb_data)
+static int mark_uninteresting(const char *UNUSED(refname),
+			      const struct object_id *oid,
+			      int UNUSED(flags),
+			      void *UNUSED(cb_data))
 {
 	struct commit *commit = lookup_commit_reference_gently(the_repository,
 							       oid, 1);
@@ -715,8 +717,10 @@ struct commit_array {
 	int nr, alloc;
 };
 
-static int add_ref(const char *refname, const struct object_id *oid,
-		   int flags, void *cb_data)
+static int add_ref(const char *UNUSED(refname),
+		   const struct object_id *oid,
+		   int UNUSED(flags),
+		   void *cb_data)
 {
 	struct commit_array *ca = cb_data;
 	ALLOC_GROW(ca->commits, ca->nr + 1, ca->alloc);
diff --git a/submodule.c b/submodule.c
index 3fa5db3ecd..d99c978fa5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -940,8 +940,9 @@ static void free_submodules_data(struct string_list *submodules)
 	string_list_clear(submodules, 1);
 }
 
-static int has_remote(const char *refname, const struct object_id *oid,
-		      int flags, void *cb_data)
+static int has_remote(const char *UNUSED(refname),
+		      const struct object_id *UNUSED(oid),
+		      int UNUSED(flags), void *UNUSED(cb_data))
 {
 	return 1;
 }
@@ -1243,8 +1244,9 @@ int push_unpushed_submodules(struct repository *r,
 	return ret;
 }
 
-static int append_oid_to_array(const char *ref, const struct object_id *oid,
-			       int flags, void *data)
+static int append_oid_to_array(const char *UNUSED(ref),
+			       const struct object_id *oid,
+			       int UNUSED(flags), void *data)
 {
 	struct oid_array *array = data;
 	oid_array_append(array, oid);
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 4d18bfb1ca..a98775d1a6 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -161,7 +161,7 @@ static int cmd_rename_ref(struct ref_store *refs, const char **argv)
 }
 
 static int each_ref(const char *refname, const struct object_id *oid,
-		    int flags, void *cb_data)
+		    int flags, void *UNUSED(cb_data))
 {
 	printf("%s %s 0x%x\n", oid_to_hex(oid), refname, flags);
 	return 0;
diff --git a/upload-pack.c b/upload-pack.c
index b217a1f469..b2cbca1e8b 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -1170,7 +1170,7 @@ static int mark_our_ref(const char *refname, const char *refname_full,
 }
 
 static int check_ref(const char *refname_full, const struct object_id *oid,
-		     int flag, void *cb_data)
+		     int UNUSED(flag), void *UNUSED(cb_data))
 {
 	const char *refname = strip_namespace(refname_full);
 
@@ -1194,7 +1194,7 @@ static void format_session_id(struct strbuf *buf, struct upload_pack_data *d) {
 }
 
 static int send_ref(const char *refname, const struct object_id *oid,
-		    int flag, void *cb_data)
+		    int UNUSED(flag), void *cb_data)
 {
 	static const char *capabilities = "multi_ack thin-pack side-band"
 		" side-band-64k ofs-delta shallow deepen-since deepen-not"
@@ -1236,7 +1236,8 @@ static int send_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int find_symref(const char *refname, const struct object_id *oid,
+static int find_symref(const char *refname,
+		       const struct object_id *UNUSED(oid),
 		       int flag, void *cb_data)
 {
 	const char *symref_target;
diff --git a/walker.c b/walker.c
index c5e2921979..f945d021f8 100644
--- a/walker.c
+++ b/walker.c
@@ -215,8 +215,10 @@ static int interpret_target(struct walker *walker, char *target, struct object_i
 	return -1;
 }
 
-static int mark_complete(const char *path, const struct object_id *oid,
-			 int flag, void *cb_data)
+static int mark_complete(const char *UNUSED(path),
+			 const struct object_id *oid,
+			 int UNUSED(flag),
+			 void *UNUSED(cb_data))
 {
 	struct commit *commit = lookup_commit_reference_gently(the_repository,
 							       oid, 1);
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 03/11] refs: mark unused reflog callback parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
  2022-08-19 10:08 ` [PATCH 01/11] git-compat-util: add UNUSED macro Jeff King
  2022-08-19 10:08 ` [PATCH 02/11] refs: mark unused each_ref_fn parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 04/11] refs: mark unused virtual method parameters Jeff King
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

Functions used with for_each_reflog_ent() need to conform to a
particular interface, but not every function needs all of the
parameters. Mark the unused ones to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/fsck.c            |  5 +++--
 builtin/stash.c           |  9 ++++++---
 commit.c                  |  5 +++--
 object-name.c             |  7 +++++--
 reflog.c                  | 13 ++++++++-----
 refs.c                    | 10 ++++++----
 remote.c                  | 15 +++++++++------
 revision.c                |  7 +++++--
 t/helper/test-ref-store.c |  2 +-
 wt-status.c               | 14 +++++++++-----
 10 files changed, 55 insertions(+), 32 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 36f1524614..31d3da8954 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -488,8 +488,9 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
 }
 
 static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid,
-		const char *email, timestamp_t timestamp, int tz,
-		const char *message, void *cb_data)
+				  const char *UNUSED(email),
+				  timestamp_t timestamp, int UNUSED(tz),
+				  const char *UNUSED(message), void *cb_data)
 {
 	const char *refname = cb_data;
 
diff --git a/builtin/stash.c b/builtin/stash.c
index 30fa101460..a741b920b3 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -638,9 +638,12 @@ static int apply_stash(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-static int reject_reflog_ent(struct object_id *ooid, struct object_id *noid,
-			     const char *email, timestamp_t timestamp, int tz,
-			     const char *message, void *cb_data)
+static int reject_reflog_ent(struct object_id *UNUSED(ooid),
+			     struct object_id *UNUSED(noid),
+			     const char *UNUSED(email),
+			     timestamp_t UNUSED(timestamp),
+			     int UNUSED(tz), const char *UNUSED(message),
+			     void *UNUSED(cb_data))
 {
 	return 1;
 }
diff --git a/commit.c b/commit.c
index 0db461f973..cb20082736 100644
--- a/commit.c
+++ b/commit.c
@@ -951,8 +951,9 @@ static void add_one_commit(struct object_id *oid, struct rev_collect *revs)
 }
 
 static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
-				  const char *ident, timestamp_t timestamp,
-				  int tz, const char *message, void *cbdata)
+				  const char *UNUSED(ident),
+				  timestamp_t UNUSED(timestamp), int UNUSED(tz),
+				  const char *UNUSED(message), void *cbdata)
 {
 	struct rev_collect *revs = cbdata;
 
diff --git a/object-name.c b/object-name.c
index 052644977e..3f7fce8322 100644
--- a/object-name.c
+++ b/object-name.c
@@ -1385,8 +1385,11 @@ struct grab_nth_branch_switch_cbdata {
 	struct strbuf *sb;
 };
 
-static int grab_nth_branch_switch(struct object_id *ooid, struct object_id *noid,
-				  const char *email, timestamp_t timestamp, int tz,
+static int grab_nth_branch_switch(struct object_id *UNUSED(ooid),
+				  struct object_id *UNUSED(noid),
+				  const char *UNUSED(email),
+				  timestamp_t UNUSED(timestamp),
+				  int UNUSED(tz),
 				  const char *message, void *cb_data)
 {
 	struct grab_nth_branch_switch_cbdata *cb = cb_data;
diff --git a/reflog.c b/reflog.c
index ee8aaa78f5..56ea3ba762 100644
--- a/reflog.c
+++ b/reflog.c
@@ -240,8 +240,9 @@ static int unreachable(struct expire_reflog_policy_cb *cb, struct commit *commit
  * Return true iff the specified reflog entry should be expired.
  */
 int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
-			     const char *email, timestamp_t timestamp, int tz,
-			     const char *message, void *cb_data)
+			     const char *UNUSED(email),
+			     timestamp_t timestamp, int UNUSED(tz),
+			     const char *UNUSED(message), void *cb_data)
 {
 	struct expire_reflog_policy_cb *cb = cb_data;
 	struct commit *old_commit, *new_commit;
@@ -379,9 +380,11 @@ void reflog_expiry_cleanup(void *cb_data)
 	}
 }
 
-int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
-		     const char *email, timestamp_t timestamp, int tz,
-		     const char *message, void *cb_data)
+int count_reflog_ent(struct object_id *UNUSED(ooid),
+		     struct object_id *UNUSED(noid),
+		     const char *UNUSED(email),
+		     timestamp_t timestamp, int UNUSED(tz),
+		     const char *UNUSED(message), void *cb_data)
 {
 	struct cmd_reflog_expire_cb *cb = cb_data;
 	if (!cb->expire_total || timestamp < cb->expire_total)
diff --git a/refs.c b/refs.c
index 34373e8087..38b1165189 100644
--- a/refs.c
+++ b/refs.c
@@ -894,8 +894,9 @@ 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 *email, timestamp_t timestamp, int tz,
-		const char *message, void *cb_data)
+			   const char *UNUSED(email),
+			   timestamp_t timestamp, int tz,
+			   const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
 	int reached_count;
@@ -950,8 +951,9 @@ 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 *email, timestamp_t timestamp,
-				  int tz, const char *message, void *cb_data)
+				  const char *UNUSED(email),
+				  timestamp_t timestamp, int tz,
+				  const char *message, void *cb_data)
 {
 	struct read_ref_at_cb *cb = cb_data;
 
diff --git a/remote.c b/remote.c
index 723aa8841c..029fc630b9 100644
--- a/remote.c
+++ b/remote.c
@@ -2577,19 +2577,22 @@ struct check_and_collect_until_cb_data {
 };
 
 /* Get the timestamp of the latest entry. */
-static int peek_reflog(struct object_id *o_oid, struct object_id *n_oid,
-		       const char *ident, timestamp_t timestamp,
-		       int tz, const char *message, void *cb_data)
+static int peek_reflog(struct object_id *UNUSED(o_oid),
+		       struct object_id *UNUSED(n_oid),
+		       const char *UNUSED(ident),
+		       timestamp_t timestamp, int UNUSED(tz),
+		       const char *UNUSED(message), void *cb_data)
 {
 	timestamp_t *ts = cb_data;
 	*ts = timestamp;
 	return 1;
 }
 
-static int check_and_collect_until(struct object_id *o_oid,
+static int check_and_collect_until(struct object_id *UNUSED(o_oid),
 				   struct object_id *n_oid,
-				   const char *ident, timestamp_t timestamp,
-				   int tz, const char *message, void *cb_data)
+				   const char *UNUSED(ident),
+				   timestamp_t timestamp, int UNUSED(tz),
+				   const char *UNUSED(message), void *cb_data)
 {
 	struct commit *commit;
 	struct check_and_collect_until_cb_data *cb = cb_data;
diff --git a/revision.c b/revision.c
index 23c2bba0d8..6c7250d6a8 100644
--- a/revision.c
+++ b/revision.c
@@ -1619,8 +1619,11 @@ static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)
 }
 
 static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
-		const char *email, timestamp_t timestamp, int tz,
-		const char *message, void *cb_data)
+				 const char *UNUSED(email),
+				 timestamp_t UNUSED(timestamp),
+				 int UNUSED(tz),
+				 const char *UNUSED(message),
+				 void *cb_data)
 {
 	handle_one_reflog_commit(ooid, cb_data);
 	handle_one_reflog_commit(noid, cb_data);
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index a98775d1a6..8f930ad358 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -207,7 +207,7 @@ static int cmd_for_each_reflog(struct ref_store *refs, const char **argv)
 
 static int each_reflog(struct object_id *old_oid, struct object_id *new_oid,
 		       const char *committer, timestamp_t timestamp,
-		       int tz, const char *msg, void *cb_data)
+		       int tz, const char *msg, void *UNUSED(cb_data))
 {
 	printf("%s %s %s %" PRItime " %+05d%s%s", oid_to_hex(old_oid),
 	       oid_to_hex(new_oid), committer, timestamp, tz,
diff --git a/wt-status.c b/wt-status.c
index 867e3e417e..38d0900aa9 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -947,9 +947,11 @@ static void wt_longstatus_print_changed(struct wt_status *s)
 	wt_longstatus_print_trailer(s);
 }
 
-static int stash_count_refs(struct object_id *ooid, struct object_id *noid,
-			    const char *email, timestamp_t timestamp, int tz,
-			    const char *message, void *cb_data)
+static int stash_count_refs(struct object_id *UNUSED(ooid),
+			    struct object_id *UNUSED(noid),
+			    const char *UNUSED(email),
+			    timestamp_t UNUSED(timestamp), int UNUSED(tz),
+			    const char *UNUSED(message), void *cb_data)
 {
 	int *c = cb_data;
 	(*c)++;
@@ -1612,8 +1614,10 @@ struct grab_1st_switch_cbdata {
 	struct object_id noid;
 };
 
-static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
-			   const char *email, timestamp_t timestamp, int tz,
+static int grab_1st_switch(struct object_id *UNUSED(ooid),
+			   struct object_id *noid,
+			   const char *UNUSED(email),
+			   timestamp_t UNUSED(timestamp), int UNUSED(tz),
 			   const char *message, void *cb_data)
 {
 	struct grab_1st_switch_cbdata *cb = cb_data;
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 04/11] refs: mark unused virtual method parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (2 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 03/11] refs: mark unused reflog callback parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 05/11] transport: mark bundle transport_options as unused Jeff King
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

The refs code uses various polymorphic types (e.g., loose vs packed
ref_stores, abstracted iterators). Not every virtual function or
callback needs all of its parameters. Let's mark the unused ones to
quiet -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 refs/files-backend.c  | 10 +++++-----
 refs/iterator.c       |  6 +++---
 refs/packed-backend.c | 14 ++++++++------
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 972701ce00..13bfdb7701 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2202,8 +2202,8 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)
 	return ok;
 }
 
-static int files_reflog_iterator_peel(struct ref_iterator *ref_iterator,
-				      struct object_id *peeled)
+static int files_reflog_iterator_peel(struct ref_iterator *UNUSED(ref_iterator),
+				      struct object_id *UNUSED(peeled))
 {
 	BUG("ref_iterator_peel() called for reflog_iterator");
 }
@@ -2257,7 +2257,7 @@ static struct ref_iterator *reflog_iterator_begin(struct ref_store *ref_store,
 static enum iterator_selection reflog_iterator_select(
 	struct ref_iterator *iter_worktree,
 	struct ref_iterator *iter_common,
-	void *cb_data)
+	void *UNUSED(cb_data))
 {
 	if (iter_worktree) {
 		/*
@@ -2985,7 +2985,7 @@ static int files_transaction_finish(struct ref_store *ref_store,
 
 static int files_transaction_abort(struct ref_store *ref_store,
 				   struct ref_transaction *transaction,
-				   struct strbuf *err)
+				   struct strbuf *UNUSED(err))
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, 0, "ref_transaction_abort");
@@ -3261,7 +3261,7 @@ static int files_reflog_expire(struct ref_store *ref_store,
 	return -1;
 }
 
-static int files_init_db(struct ref_store *ref_store, struct strbuf *err)
+static int files_init_db(struct ref_store *ref_store, struct strbuf *UNUSED(err))
 {
 	struct files_ref_store *refs =
 		files_downcast(ref_store, REF_STORE_WRITE, "init_db");
diff --git a/refs/iterator.c b/refs/iterator.c
index b2e56bae1c..e34921db72 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -51,8 +51,8 @@ static int empty_ref_iterator_advance(struct ref_iterator *ref_iterator)
 	return ref_iterator_abort(ref_iterator);
 }
 
-static int empty_ref_iterator_peel(struct ref_iterator *ref_iterator,
-				   struct object_id *peeled)
+static int empty_ref_iterator_peel(struct ref_iterator *UNUSED(ref_iterator),
+				   struct object_id *UNUSED(peeled))
 {
 	BUG("peel called for empty iterator");
 }
@@ -238,7 +238,7 @@ struct ref_iterator *merge_ref_iterator_begin(
  */
 static enum iterator_selection overlay_iterator_select(
 		struct ref_iterator *front, struct ref_iterator *back,
-		void *cb_data)
+		void *UNUSED(cb_data))
 {
 	int cmp;
 
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 97b6837767..a45bb686f0 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -726,7 +726,7 @@ static struct snapshot *get_snapshot(struct packed_ref_store *refs)
 }
 
 static int packed_read_raw_ref(struct ref_store *ref_store, const char *refname,
-			       struct object_id *oid, struct strbuf *referent,
+			       struct object_id *oid, struct strbuf *UNUSED(referent),
 			       unsigned int *type, int *failure_errno)
 {
 	struct packed_ref_store *refs =
@@ -1078,7 +1078,8 @@ int packed_refs_is_locked(struct ref_store *ref_store)
 static const char PACKED_REFS_HEADER[] =
 	"# pack-refs with: peeled fully-peeled sorted \n";
 
-static int packed_init_db(struct ref_store *ref_store, struct strbuf *err)
+static int packed_init_db(struct ref_store *UNUSED(ref_store),
+			  struct strbuf *UNUSED(err))
 {
 	/* Nothing to do. */
 	return 0;
@@ -1473,7 +1474,7 @@ static int packed_transaction_prepare(struct ref_store *ref_store,
 
 static int packed_transaction_abort(struct ref_store *ref_store,
 				    struct ref_transaction *transaction,
-				    struct strbuf *err)
+				    struct strbuf *UNUSED(err))
 {
 	struct packed_ref_store *refs = packed_downcast(
 			ref_store,
@@ -1512,7 +1513,7 @@ static int packed_transaction_finish(struct ref_store *ref_store,
 	return ret;
 }
 
-static int packed_initial_transaction_commit(struct ref_store *ref_store,
+static int packed_initial_transaction_commit(struct ref_store *UNUSED(ref_store),
 					    struct ref_transaction *transaction,
 					    struct strbuf *err)
 {
@@ -1568,7 +1569,8 @@ static int packed_delete_refs(struct ref_store *ref_store, const char *msg,
 	return ret;
 }
 
-static int packed_pack_refs(struct ref_store *ref_store, unsigned int flags)
+static int packed_pack_refs(struct ref_store *UNUSED(ref_store),
+			    unsigned int UNUSED(flags))
 {
 	/*
 	 * Packed refs are already packed. It might be that loose refs
@@ -1578,7 +1580,7 @@ static int packed_pack_refs(struct ref_store *ref_store, unsigned int flags)
 	return 0;
 }
 
-static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store)
+static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *UNUSED(ref_store))
 {
 	return empty_ref_iterator_begin();
 }
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 05/11] transport: mark bundle transport_options as unused
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (3 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 04/11] refs: mark unused virtual method parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 06/11] streaming: mark unused virtual method parameters Jeff King
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

get_refs_from_bundle() is a virtual function which must match the
signature of other transports, but it doesn't look at its
transport_options at all. This isn't a bug, because not all transports
necessarily support all options. Let's mark it as unused to appease
-Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.c b/transport.c
index b51e991e44..551cad22dd 100644
--- a/transport.c
+++ b/transport.c
@@ -142,7 +142,7 @@ static void get_refs_from_bundle_inner(struct transport *transport)
 
 static struct ref *get_refs_from_bundle(struct transport *transport,
 					int for_push,
-					struct transport_ls_refs_options *transport_options)
+					struct transport_ls_refs_options *UNUSED(transport_options))
 {
 	struct bundle_transport_data *data = transport->data;
 	struct ref *result = NULL;
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 06/11] streaming: mark unused virtual method parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (4 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 05/11] transport: mark bundle transport_options as unused Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 07/11] config: mark unused callback parameters Jeff King
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

Streaming "open" functions need to conform to the same virtual function
interface, but not every implementation needs every parameter. Mark the
unused ones as such to appease -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 streaming.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/streaming.c b/streaming.c
index fe54665d86..4b34e2a748 100644
--- a/streaming.c
+++ b/streaming.c
@@ -328,9 +328,9 @@ static int close_istream_pack_non_delta(struct git_istream *st)
 }
 
 static int open_istream_pack_non_delta(struct git_istream *st,
-				       struct repository *r,
-				       const struct object_id *oid,
-				       enum object_type *type)
+				       struct repository *UNUSED(r),
+				       const struct object_id *UNUSED(oid),
+				       enum object_type *UNUSED(type))
 {
 	struct pack_window *window;
 	enum object_type in_pack_type;
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 07/11] config: mark unused callback parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (5 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 06/11] streaming: mark unused virtual method parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 08/11] hashmap: " Jeff King
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

The callback passed to git_config() must conform to a particular
interface. But most callbacks don't actually look at the extra "void
*data" parameter. Let's mark the unused parameters to make
-Wunused-parameter happy.

Note there's one unusual case here in get_remote_default() where we
actually ignore the "value" parameter. That's because it's only checking
whether the option is found at all, and not parsing its value.

Signed-off-by: Jeff King <peff@peff.net>
---
 archive-tar.c              | 3 ++-
 archive-zip.c              | 3 ++-
 builtin/am.c               | 2 +-
 builtin/commit-graph.c     | 2 +-
 builtin/config.c           | 8 +++++---
 builtin/multi-pack-index.c | 2 +-
 builtin/remote.c           | 2 +-
 color.c                    | 2 +-
 config.c                   | 3 ++-
 convert.c                  | 2 +-
 delta-islands.c            | 2 +-
 diff.c                     | 3 ++-
 git-compat-util.h          | 4 +++-
 gpg-interface.c            | 2 +-
 ident.c                    | 2 +-
 ll-merge.c                 | 3 ++-
 ls-refs.c                  | 3 ++-
 pager.c                    | 3 ++-
 pretty.c                   | 3 ++-
 submodule.c                | 3 ++-
 t/helper/test-config.c     | 2 +-
 t/helper/test-userdiff.c   | 2 +-
 trailer.c                  | 6 ++++--
 23 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 3d77e0f750..45e5d91407 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -366,7 +366,8 @@ static struct archiver *find_tar_filter(const char *name, size_t len)
 	return NULL;
 }
 
-static int tar_filter_config(const char *var, const char *value, void *data)
+static int tar_filter_config(const char *var, const char *value,
+			     void *UNUSED(data))
 {
 	struct archiver *ar;
 	const char *name;
diff --git a/archive-zip.c b/archive-zip.c
index 9fe43d740d..854bceb018 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -612,7 +612,8 @@ static void dos_time(timestamp_t *timestamp, int *dos_date, int *dos_time)
 	*dos_time = tm.tm_sec / 2 + tm.tm_min * 32 + tm.tm_hour * 2048;
 }
 
-static int archive_zip_config(const char *var, const char *value, void *data)
+static int archive_zip_config(const char *var, const char *value,
+			      void *UNUSED(data))
 {
 	return userdiff_config(var, value);
 }
diff --git a/builtin/am.c b/builtin/am.c
index 93bec62afa..0811b9ff67 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2301,7 +2301,7 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar
 	return 0;
 }
 
-static int git_am_config(const char *k, const char *v, void *cb)
+static int git_am_config(const char *k, const char *v, void *UNUSED(cb))
 {
 	int status;
 
diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
index 51c4040ea6..ea923ea33a 100644
--- a/builtin/commit-graph.c
+++ b/builtin/commit-graph.c
@@ -179,7 +179,7 @@ static int write_option_max_new_filters(const struct option *opt,
 }
 
 static int git_commit_graph_write_config(const char *var, const char *value,
-					 void *cb)
+					 void *UNUSED(cb))
 {
 	if (!strcmp(var, "commitgraph.maxnewfilters"))
 		write_opts.max_new_filters = git_config_int(var, value);
diff --git a/builtin/config.c b/builtin/config.c
index e7b88a9c08..bdc8b1d1a8 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -207,7 +207,8 @@ static void show_config_scope(struct strbuf *buf)
 	strbuf_addch(buf, term);
 }
 
-static int show_all_config(const char *key_, const char *value_, void *cb)
+static int show_all_config(const char *key_, const char *value_,
+			   void *UNUSED(cb))
 {
 	if (show_origin || show_scope) {
 		struct strbuf buf = STRBUF_INIT;
@@ -458,7 +459,8 @@ static const char *get_color_slot;
 static const char *get_colorbool_slot;
 static char parsed_color[COLOR_MAXLEN];
 
-static int git_get_color_config(const char *var, const char *value, void *cb)
+static int git_get_color_config(const char *var, const char *value,
+				void *UNUSED(cb))
 {
 	if (!strcmp(var, get_color_slot)) {
 		if (!value)
@@ -490,7 +492,7 @@ static int get_colorbool_found;
 static int get_diff_color_found;
 static int get_color_ui_found;
 static int git_get_colorbool_config(const char *var, const char *value,
-		void *cb)
+				    void *UNUSED(data))
 {
 	if (!strcmp(var, get_colorbool_slot))
 		get_colorbool_found = git_config_colorbool(var, value);
diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
index 8f24d59a75..8d156766af 100644
--- a/builtin/multi-pack-index.c
+++ b/builtin/multi-pack-index.c
@@ -78,7 +78,7 @@ static struct option *add_common_options(struct option *prev)
 }
 
 static int git_multi_pack_index_write_config(const char *var, const char *value,
-					     void *cb)
+					     void *UNUSED(cb))
 {
 	if (!strcmp(var, "pack.writebitmaphashcache")) {
 		if (git_config_bool(var, value))
diff --git a/builtin/remote.c b/builtin/remote.c
index b390360f07..87dda7c37b 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1486,7 +1486,7 @@ static int prune(int argc, const char **argv)
 	return result;
 }
 
-static int get_remote_default(const char *key, const char *value, void *priv)
+static int get_remote_default(const char *key, const char *UNUSED(value), void *priv)
 {
 	if (strcmp(key, "remotes.default") == 0) {
 		int *found = priv;
diff --git a/color.c b/color.c
index 4f884c6b3d..04ad0a8bf7 100644
--- a/color.c
+++ b/color.c
@@ -415,7 +415,7 @@ int want_color_fd(int fd, int var)
 	return var;
 }
 
-int git_color_config(const char *var, const char *value, void *cb)
+int git_color_config(const char *var, const char *value, void *UNUSED(cb))
 {
 	if (!strcmp(var, "color.ui")) {
 		git_use_color_default = git_config_colorbool(var, value);
diff --git a/config.c b/config.c
index e8ebef77d5..589dec9028 100644
--- a/config.c
+++ b/config.c
@@ -362,7 +362,8 @@ static void populate_remote_urls(struct config_include_data *inc)
 	current_parsing_scope = store_scope;
 }
 
-static int forbid_remote_url(const char *var, const char *value, void *data)
+static int forbid_remote_url(const char *var, const char *UNUSED(value),
+			     void *UNUSED(data))
 {
 	const char *remote_name;
 	size_t remote_name_len;
diff --git a/convert.c b/convert.c
index 4d153729da..b31a25b536 100644
--- a/convert.c
+++ b/convert.c
@@ -1008,7 +1008,7 @@ static int apply_filter(const char *path, const char *src, size_t len,
 	return 0;
 }
 
-static int read_convert_config(const char *var, const char *value, void *cb)
+static int read_convert_config(const char *var, const char *value, void *UNUSED(cb))
 {
 	const char *key, *name;
 	size_t namelen;
diff --git a/delta-islands.c b/delta-islands.c
index 13eb96e0c4..c64333f9de 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -316,7 +316,7 @@ static regex_t *island_regexes;
 static unsigned int island_regexes_alloc, island_regexes_nr;
 static const char *core_island_name;
 
-static int island_config_callback(const char *k, const char *v, void *cb)
+static int island_config_callback(const char *k, const char *v, void *UNUSED(cb))
 {
 	if (!strcmp(k, "pack.island")) {
 		struct strbuf re = STRBUF_INIT;
diff --git a/diff.c b/diff.c
index 974626a621..8a9c9083f3 100644
--- a/diff.c
+++ b/diff.c
@@ -264,7 +264,8 @@ void init_diff_ui_defaults(void)
 	diff_detect_rename_default = DIFF_DETECT_RENAME;
 }
 
-int git_diff_heuristic_config(const char *var, const char *value, void *cb)
+int git_diff_heuristic_config(const char *var, const char *value,
+			      void *UNUSED(cb))
 {
 	if (!strcmp(var, "diff.indentheuristic"))
 		diff_indent_heuristic = git_config_bool(var, value);
diff --git a/git-compat-util.h b/git-compat-util.h
index c6669db07d..12239fedf7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -403,7 +403,9 @@ typedef uintmax_t timestamp_t;
 #endif
 
 #ifndef platform_core_config
-static inline int noop_core_config(const char *var, const char *value, void *cb)
+static inline int noop_core_config(const char *UNUSED(var),
+				   const char *UNUSED(value),
+				   void *UNUSED(cb))
 {
 	return 0;
 }
diff --git a/gpg-interface.c b/gpg-interface.c
index 6dff241460..721d69bf42 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -699,7 +699,7 @@ void set_signing_key(const char *key)
 	configured_signing_key = xstrdup(key);
 }
 
-int git_gpg_config(const char *var, const char *value, void *cb)
+int git_gpg_config(const char *var, const char *value, void *UNUSED(cb))
 {
 	struct gpg_format *fmt = NULL;
 	char *fmtname = NULL;
diff --git a/ident.c b/ident.c
index 7f66beda42..48745a1f0e 100644
--- a/ident.c
+++ b/ident.c
@@ -668,7 +668,7 @@ static int set_ident(const char *var, const char *value)
 	return 0;
 }
 
-int git_ident_config(const char *var, const char *value, void *data)
+int git_ident_config(const char *var, const char *value, void *UNUSED(data))
 {
 	if (!strcmp(var, "user.useconfigonly")) {
 		ident_use_config_only = git_config_bool(var, value);
diff --git a/ll-merge.c b/ll-merge.c
index 14b8362019..9f3ae1f8fe 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -249,7 +249,8 @@ static enum ll_merge_result ll_ext_merge(const struct ll_merge_driver *fn,
 static struct ll_merge_driver *ll_user_merge, **ll_user_merge_tail;
 static const char *default_ll_merge;
 
-static int read_merge_config(const char *var, const char *value, void *cb)
+static int read_merge_config(const char *var, const char *value,
+			     void *UNUSED(cb))
 {
 	struct ll_merge_driver *fn;
 	const char *key, *name;
diff --git a/ls-refs.c b/ls-refs.c
index 98e69373c8..e54b883e87 100644
--- a/ls-refs.c
+++ b/ls-refs.c
@@ -136,7 +136,8 @@ static void send_possibly_unborn_head(struct ls_refs_data *data)
 	strbuf_release(&namespaced);
 }
 
-static int ls_refs_config(const char *var, const char *value, void *data)
+static int ls_refs_config(const char *var, const char *value,
+			  void *UNUSED(data))
 {
 	/*
 	 * We only serve fetches over v2 for now, so respect only "uploadpack"
diff --git a/pager.c b/pager.c
index 5cfe23b025..19c016ff54 100644
--- a/pager.c
+++ b/pager.c
@@ -38,7 +38,8 @@ static void wait_for_pager_signal(int signo)
 	raise(signo);
 }
 
-static int core_pager_config(const char *var, const char *value, void *data)
+static int core_pager_config(const char *var, const char *value,
+			     void *UNUSED(data))
 {
 	if (!strcmp(var, "core.pager"))
 		return git_config_string(&pager_program, var, value);
diff --git a/pretty.c b/pretty.c
index 6d819103fb..584026b746 100644
--- a/pretty.c
+++ b/pretty.c
@@ -43,7 +43,8 @@ static void save_user_format(struct rev_info *rev, const char *cp, int is_tforma
 	rev->commit_format = CMIT_FMT_USERFORMAT;
 }
 
-static int git_pretty_formats_config(const char *var, const char *value, void *cb)
+static int git_pretty_formats_config(const char *var, const char *value,
+				     void *UNUSED(cb))
 {
 	struct cmt_fmt_map *commit_format = NULL;
 	const char *name;
diff --git a/submodule.c b/submodule.c
index d99c978fa5..597a347f85 100644
--- a/submodule.c
+++ b/submodule.c
@@ -213,7 +213,8 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,
 }
 
 /* Cheap function that only determines if we're interested in submodules at all */
-int git_default_submodule_config(const char *var, const char *value, void *cb)
+int git_default_submodule_config(const char *var, const char *value,
+				 void *UNUSED(cb))
 {
 	if (!strcmp(var, "submodule.recurse")) {
 		int v = git_config_bool(var, value) ?
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index a6e936721f..ddd538b838 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -37,7 +37,7 @@
  *
  */
 
-static int iterate_cb(const char *var, const char *value, void *data)
+static int iterate_cb(const char *var, const char *value, void *UNUSED(data))
 {
 	static int nr;
 
diff --git a/t/helper/test-userdiff.c b/t/helper/test-userdiff.c
index f013f8a31e..64538a0c20 100644
--- a/t/helper/test-userdiff.c
+++ b/t/helper/test-userdiff.c
@@ -12,7 +12,7 @@ static int driver_cb(struct userdiff_driver *driver,
 	return 0;
 }
 
-static int cmd__userdiff_config(const char *var, const char *value, void *cb)
+static int cmd__userdiff_config(const char *var, const char *value, void *UNUSED(cb))
 {
 	if (userdiff_config(var, value) < 0)
 		return -1;
diff --git a/trailer.c b/trailer.c
index d419c20735..a1e80478ab 100644
--- a/trailer.c
+++ b/trailer.c
@@ -478,7 +478,8 @@ static struct {
 	{ "ifmissing", TRAILER_IF_MISSING }
 };
 
-static int git_trailer_default_config(const char *conf_key, const char *value, void *cb)
+static int git_trailer_default_config(const char *conf_key, const char *value,
+				      void *UNUSED(cb))
 {
 	const char *trailer_item, *variable_name;
 
@@ -509,7 +510,8 @@ static int git_trailer_default_config(const char *conf_key, const char *value, v
 	return 0;
 }
 
-static int git_trailer_config(const char *conf_key, const char *value, void *cb)
+static int git_trailer_config(const char *conf_key, const char *value,
+			      void *UNUSED(cb))
 {
 	const char *trailer_item, *variable_name;
 	struct arg_item *item;
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 08/11] hashmap: mark unused callback parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (6 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 07/11] config: mark unused callback parameters Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 09/11] mark unused read_tree_recursive() " Jeff King
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

Hashmap comparison functions must conform to a particular callback
interface, but many don't use all of their parameters. Especially the
void cmp_data pointer, but some do not use keydata either (because they
can easily form a full struct to pass when doing lookups). Let's mark
these to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
---
 add-interactive.c     |  2 +-
 attr.c                |  4 ++--
 bloom.c               |  4 ++--
 builtin/describe.c    |  2 +-
 builtin/difftool.c    | 10 +++++-----
 builtin/fast-export.c |  2 +-
 builtin/fast-import.c |  2 +-
 builtin/fetch.c       |  2 +-
 compat/terminal.c     |  2 +-
 config.c              |  4 ++--
 diff.c                |  2 +-
 dir.c                 |  4 ++--
 environment.c         |  4 ++--
 hashmap.c             | 10 +++++-----
 merge-recursive.c     | 10 +++++-----
 name-hash.c           |  4 ++--
 object-store.h        |  2 +-
 oidmap.c              |  2 +-
 packfile.c            |  2 +-
 patch-ids.c           |  2 +-
 range-diff.c          |  6 ++++--
 ref-filter.c          |  2 +-
 refs.c                |  2 +-
 remote.c              |  4 ++--
 revision.c            |  4 ++--
 sequencer.c           |  5 +++--
 strmap.c              |  4 ++--
 sub-process.c         |  4 ++--
 submodule-config.c    |  8 ++++----
 29 files changed, 59 insertions(+), 56 deletions(-)

diff --git a/add-interactive.c b/add-interactive.c
index 22fcd3412c..2fcad67654 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -430,7 +430,7 @@ struct pathname_entry {
 	struct file_item *item;
 };
 
-static int pathname_entry_cmp(const void *unused_cmp_data,
+static int pathname_entry_cmp(const void *UNUSED(cmp_data),
 			      const struct hashmap_entry *he1,
 			      const struct hashmap_entry *he2,
 			      const void *name)
diff --git a/attr.c b/attr.c
index 8a78dde69e..63e1a756dd 100644
--- a/attr.c
+++ b/attr.c
@@ -61,10 +61,10 @@ struct attr_hash_entry {
 };
 
 /* attr_hashmap comparison function */
-static int attr_hash_entry_cmp(const void *unused_cmp_data,
+static int attr_hash_entry_cmp(const void *UNUSED(cmp_data),
 			       const struct hashmap_entry *eptr,
 			       const struct hashmap_entry *entry_or_key,
-			       const void *unused_keydata)
+			       const void *UNUSED(keydata))
 {
 	const struct attr_hash_entry *a, *b;
 
diff --git a/bloom.c b/bloom.c
index 816f063dca..94fb97e60e 100644
--- a/bloom.c
+++ b/bloom.c
@@ -163,10 +163,10 @@ void init_bloom_filters(void)
 	init_bloom_filter_slab(&bloom_filters);
 }
 
-static int pathmap_cmp(const void *hashmap_cmp_fn_data,
+static int pathmap_cmp(const void *UNUSED(hashmap_cmp_fn_data),
 		       const struct hashmap_entry *eptr,
 		       const struct hashmap_entry *entry_or_key,
-		       const void *keydata)
+		       const void *UNUSED(keydata))
 {
 	const struct pathmap_hash_entry *e1, *e2;
 
diff --git a/builtin/describe.c b/builtin/describe.c
index 3af36483f2..084fa00f2a 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -63,7 +63,7 @@ static const char *prio_names[] = {
 	N_("head"), N_("lightweight"), N_("annotated"),
 };
 
-static int commit_name_neq(const void *unused_cmp_data,
+static int commit_name_neq(const void *UNUSED(cmp_data),
 			   const struct hashmap_entry *eptr,
 			   const struct hashmap_entry *entry_or_key,
 			   const void *peeled)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index b3c509b8de..a570200e66 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -125,10 +125,10 @@ struct working_tree_entry {
 	char path[FLEX_ARRAY];
 };
 
-static int working_tree_entry_cmp(const void *unused_cmp_data,
+static int working_tree_entry_cmp(const void *UNUSED(cmp_data),
 				  const struct hashmap_entry *eptr,
 				  const struct hashmap_entry *entry_or_key,
-				  const void *unused_keydata)
+				  const void *UNUSED(keydata))
 {
 	const struct working_tree_entry *a, *b;
 
@@ -148,10 +148,10 @@ struct pair_entry {
 	const char path[FLEX_ARRAY];
 };
 
-static int pair_cmp(const void *unused_cmp_data,
+static int pair_cmp(const void *UNUSED(cmp_data),
 		    const struct hashmap_entry *eptr,
 		    const struct hashmap_entry *entry_or_key,
-		    const void *unused_keydata)
+		    const void *UNUSED(keydata))
 {
 	const struct pair_entry *a, *b;
 
@@ -184,7 +184,7 @@ struct path_entry {
 	char path[FLEX_ARRAY];
 };
 
-static int path_entry_cmp(const void *unused_cmp_data,
+static int path_entry_cmp(const void *UNUSED(cmp_data),
 			  const struct hashmap_entry *eptr,
 			  const struct hashmap_entry *entry_or_key,
 			  const void *key)
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index e1748fb98b..bb05b50a5a 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -119,7 +119,7 @@ struct anonymized_entry_key {
 	size_t orig_len;
 };
 
-static int anonymized_entry_cmp(const void *unused_cmp_data,
+static int anonymized_entry_cmp(const void *UNUSED(cmp_data),
 				const struct hashmap_entry *eptr,
 				const struct hashmap_entry *entry_or_key,
 				const void *keydata)
diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 14113cfd82..76ed0c2db9 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -46,7 +46,7 @@ struct object_entry {
 		depth : DEPTH_BITS;
 };
 
-static int object_entry_hashcmp(const void *map_data,
+static int object_entry_hashcmp(const void *UNUSED(map_data),
 				const struct hashmap_entry *eptr,
 				const struct hashmap_entry *entry_or_key,
 				const void *keydata)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 9e7c8099fe..5fddaef480 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -301,7 +301,7 @@ struct refname_hash_entry {
 	char refname[FLEX_ARRAY];
 };
 
-static int refname_hash_entry_cmp(const void *hashmap_cmp_fn_data,
+static int refname_hash_entry_cmp(const void *UNUSED(hashmap_cmp_fn_data),
 				  const struct hashmap_entry *eptr,
 				  const struct hashmap_entry *entry_or_key,
 				  const void *keydata)
diff --git a/compat/terminal.c b/compat/terminal.c
index 7db330c52d..0b0caae857 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -477,7 +477,7 @@ struct escape_sequence_entry {
 	char sequence[FLEX_ARRAY];
 };
 
-static int sequence_entry_cmp(const void *hashmap_cmp_fn_data,
+static int sequence_entry_cmp(const void *UNUSED(hashmap_cmp_fn_data),
 			      const struct escape_sequence_entry *e1,
 			      const struct escape_sequence_entry *e2,
 			      const void *keydata)
diff --git a/config.c b/config.c
index 589dec9028..c92f1efd6b 100644
--- a/config.c
+++ b/config.c
@@ -2338,10 +2338,10 @@ static int configset_add_value(struct config_set *cs, const char *key, const cha
 	return 0;
 }
 
-static int config_set_element_cmp(const void *unused_cmp_data,
+static int config_set_element_cmp(const void *UNUSED(cmp_data),
 				  const struct hashmap_entry *eptr,
 				  const struct hashmap_entry *entry_or_key,
-				  const void *unused_keydata)
+				  const void *UNUSED(keydata))
 {
 	const struct config_set_element *e1, *e2;
 
diff --git a/diff.c b/diff.c
index 8a9c9083f3..7c7d53d277 100644
--- a/diff.c
+++ b/diff.c
@@ -917,7 +917,7 @@ struct interned_diff_symbol {
 static int interned_diff_symbol_cmp(const void *hashmap_cmp_fn_data,
 				    const struct hashmap_entry *eptr,
 				    const struct hashmap_entry *entry_or_key,
-				    const void *keydata)
+				    const void *UNUSED(keydata))
 {
 	const struct diff_options *diffopt = hashmap_cmp_fn_data;
 	const struct emitted_diff_symbol *a, *b;
diff --git a/dir.c b/dir.c
index 50eeb8b11e..e8b225eed4 100644
--- a/dir.c
+++ b/dir.c
@@ -655,10 +655,10 @@ void parse_path_pattern(const char **pattern,
 	*patternlen = len;
 }
 
-int pl_hashmap_cmp(const void *unused_cmp_data,
+int pl_hashmap_cmp(const void *UNUSED(cmp_data),
 		   const struct hashmap_entry *a,
 		   const struct hashmap_entry *b,
-		   const void *key)
+		   const void *UNUSED(key))
 {
 	const struct pattern_entry *ee1 =
 			container_of(a, struct pattern_entry, ent);
diff --git a/environment.c b/environment.c
index b3296ce7d1..a0ceb6d655 100644
--- a/environment.c
+++ b/environment.c
@@ -333,10 +333,10 @@ static void set_git_dir_1(const char *path)
 	setup_git_env(path);
 }
 
-static void update_relative_gitdir(const char *name,
+static void update_relative_gitdir(const char *UNUSED(name),
 				   const char *old_cwd,
 				   const char *new_cwd,
-				   void *data)
+				   void *UNUSED(data))
 {
 	char *path = reparent_relative_path(old_cwd, new_cwd, get_git_dir());
 	struct tmp_objdir *tmp_objdir = tmp_objdir_unapply_primary_odb();
diff --git a/hashmap.c b/hashmap.c
index 134d2eec80..763aa1d8a3 100644
--- a/hashmap.c
+++ b/hashmap.c
@@ -142,10 +142,10 @@ static inline struct hashmap_entry **find_entry_ptr(const struct hashmap *map,
 	return e;
 }
 
-static int always_equal(const void *unused_cmp_data,
-			const struct hashmap_entry *unused1,
-			const struct hashmap_entry *unused2,
-			const void *unused_keydata)
+static int always_equal(const void *UNUSED(cmp_data),
+			const struct hashmap_entry *UNUSED(entry1),
+			const struct hashmap_entry *UNUSED(entry2),
+			const void *UNUSED(keydata))
 {
 	return 0;
 }
@@ -313,7 +313,7 @@ struct pool_entry {
 	unsigned char data[FLEX_ARRAY];
 };
 
-static int pool_entry_cmp(const void *unused_cmp_data,
+static int pool_entry_cmp(const void *UNUSED(cmp_data),
 			  const struct hashmap_entry *eptr,
 			  const struct hashmap_entry *entry_or_key,
 			  const void *keydata)
diff --git a/merge-recursive.c b/merge-recursive.c
index b83a129b43..775ebe2182 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -45,7 +45,7 @@ struct path_hashmap_entry {
 	char path[FLEX_ARRAY];
 };
 
-static int path_hashmap_cmp(const void *cmp_data,
+static int path_hashmap_cmp(const void *UNUSED(cmp_data),
 			    const struct hashmap_entry *eptr,
 			    const struct hashmap_entry *entry_or_key,
 			    const void *keydata)
@@ -89,10 +89,10 @@ static struct dir_rename_entry *dir_rename_find_entry(struct hashmap *hashmap,
 	return hashmap_get_entry(hashmap, &key, ent, NULL);
 }
 
-static int dir_rename_cmp(const void *unused_cmp_data,
+static int dir_rename_cmp(const void *UNUSED(cmp_data),
 			  const struct hashmap_entry *eptr,
 			  const struct hashmap_entry *entry_or_key,
-			  const void *unused_keydata)
+			  const void *UNUSED(keydata))
 {
 	const struct dir_rename_entry *e1, *e2;
 
@@ -134,10 +134,10 @@ static struct collision_entry *collision_find_entry(struct hashmap *hashmap,
 	return hashmap_get_entry(hashmap, &key, ent, NULL);
 }
 
-static int collision_cmp(const void *unused_cmp_data,
+static int collision_cmp(const void *UNUSED(cmp_data),
 			 const struct hashmap_entry *eptr,
 			 const struct hashmap_entry *entry_or_key,
-			 const void *unused_keydata)
+			 const void *UNUSED(keydata))
 {
 	const struct collision_entry *e1, *e2;
 
diff --git a/name-hash.c b/name-hash.c
index 7487d33124..d0da6db564 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -18,7 +18,7 @@ struct dir_entry {
 	char name[FLEX_ARRAY];
 };
 
-static int dir_entry_cmp(const void *unused_cmp_data,
+static int dir_entry_cmp(const void *UNUSED(cmp_data),
 			 const struct hashmap_entry *eptr,
 			 const struct hashmap_entry *entry_or_key,
 			 const void *keydata)
@@ -120,7 +120,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
 		add_dir_entry(istate, ce);
 }
 
-static int cache_entry_cmp(const void *unused_cmp_data,
+static int cache_entry_cmp(const void *UNUSED(cmp_data),
 			   const struct hashmap_entry *eptr,
 			   const struct hashmap_entry *entry_or_key,
 			   const void *remove)
diff --git a/object-store.h b/object-store.h
index 5222ee5460..cf5494af80 100644
--- a/object-store.h
+++ b/object-store.h
@@ -141,7 +141,7 @@ struct packed_git {
 
 struct multi_pack_index;
 
-static inline int pack_map_entry_cmp(const void *unused_cmp_data,
+static inline int pack_map_entry_cmp(const void *UNUSED(cmp_data),
 				     const struct hashmap_entry *entry,
 				     const struct hashmap_entry *entry2,
 				     const void *keydata)
diff --git a/oidmap.c b/oidmap.c
index 286a04a53c..32aeb0526f 100644
--- a/oidmap.c
+++ b/oidmap.c
@@ -1,7 +1,7 @@
 #include "cache.h"
 #include "oidmap.h"
 
-static int oidmap_neq(const void *hashmap_cmp_fn_data,
+static int oidmap_neq(const void *UNUSED(hashmap_cmp_fn_data),
 		      const struct hashmap_entry *e1,
 		      const struct hashmap_entry *e2,
 		      const void *keydata)
diff --git a/packfile.c b/packfile.c
index 5ae3ce8ea9..d7cf8382de 100644
--- a/packfile.c
+++ b/packfile.c
@@ -1392,7 +1392,7 @@ static int delta_base_cache_key_eq(const struct delta_base_cache_key *a,
 	return a->p == b->p && a->base_offset == b->base_offset;
 }
 
-static int delta_base_cache_hash_cmp(const void *unused_cmp_data,
+static int delta_base_cache_hash_cmp(const void *UNUSED(cmp_data),
 				     const struct hashmap_entry *va,
 				     const struct hashmap_entry *vb,
 				     const void *vkey)
diff --git a/patch-ids.c b/patch-ids.c
index 8bf425555d..cdfa513549 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -38,7 +38,7 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
 static int patch_id_neq(const void *cmpfn_data,
 			const struct hashmap_entry *eptr,
 			const struct hashmap_entry *entry_or_key,
-			const void *unused_keydata)
+			const void *UNUSED(keydata))
 {
 	/* NEEDSWORK: const correctness? */
 	struct diff_options *opt = (void *)cmpfn_data;
diff --git a/range-diff.c b/range-diff.c
index f63b3ffc20..1528fdd0db 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -224,8 +224,10 @@ static int read_patches(const char *range, struct string_list *list,
 	return ret;
 }
 
-static int patch_util_cmp(const void *dummy, const struct patch_util *a,
-			  const struct patch_util *b, const char *keydata)
+static int patch_util_cmp(const void *UNUSED(cmp_data),
+			  const struct patch_util *a,
+			  const struct patch_util *b,
+			  const char *keydata)
 {
 	return strcmp(a->diff, keydata ? keydata : b->diff);
 }
diff --git a/ref-filter.c b/ref-filter.c
index bdf39fa761..baf252b77d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -89,7 +89,7 @@ struct ref_to_worktree_entry {
 	struct worktree *wt; /* key is wt->head_ref */
 };
 
-static int ref_to_worktree_map_cmpfnc(const void *unused_lookupdata,
+static int ref_to_worktree_map_cmpfnc(const void *UNUSED(lookupdata),
 				      const struct hashmap_entry *eptr,
 				      const struct hashmap_entry *kptr,
 				      const void *keydata_aka_refname)
diff --git a/refs.c b/refs.c
index 38b1165189..5012bba357 100644
--- a/refs.c
+++ b/refs.c
@@ -1815,7 +1815,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 *UNUSED(cmp_data),
 			      const struct hashmap_entry *eptr,
 			      const struct hashmap_entry *entry_or_key,
 			      const void *keydata)
diff --git a/remote.c b/remote.c
index 029fc630b9..ef12aba91d 100644
--- a/remote.c
+++ b/remote.c
@@ -86,7 +86,7 @@ struct remotes_hash_key {
 	int len;
 };
 
-static int remotes_hash_cmp(const void *unused_cmp_data,
+static int remotes_hash_cmp(const void *UNUSED(cmp_data),
 			    const struct hashmap_entry *eptr,
 			    const struct hashmap_entry *entry_or_key,
 			    const void *keydata)
@@ -170,7 +170,7 @@ struct branches_hash_key {
 	int len;
 };
 
-static int branches_hash_cmp(const void *unused_cmp_data,
+static int branches_hash_cmp(const void *UNUSED(cmp_data),
 			     const struct hashmap_entry *eptr,
 			     const struct hashmap_entry *entry_or_key,
 			     const void *keydata)
diff --git a/revision.c b/revision.c
index 6c7250d6a8..5eb71e32d0 100644
--- a/revision.c
+++ b/revision.c
@@ -119,10 +119,10 @@ struct path_and_oids_entry {
 	struct oidset trees;
 };
 
-static int path_and_oids_cmp(const void *hashmap_cmp_fn_data,
+static int path_and_oids_cmp(const void *UNUSED(hashmap_cmp_fn_data),
 			     const struct hashmap_entry *eptr,
 			     const struct hashmap_entry *entry_or_key,
-			     const void *keydata)
+			     const void *UNUSED(keydata))
 {
 	const struct path_and_oids_entry *e1, *e2;
 
diff --git a/sequencer.c b/sequencer.c
index 5f22b7cd37..e5b52651f8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5254,7 +5254,8 @@ struct labels_entry {
 	char label[FLEX_ARRAY];
 };
 
-static int labels_cmp(const void *fndata, const struct hashmap_entry *eptr,
+static int labels_cmp(const void *UNUSED(fndata),
+		      const struct hashmap_entry *eptr,
 		      const struct hashmap_entry *entry_or_key, const void *key)
 {
 	const struct labels_entry *a, *b;
@@ -6131,7 +6132,7 @@ struct subject2item_entry {
 	char subject[FLEX_ARRAY];
 };
 
-static int subject2item_cmp(const void *fndata,
+static int subject2item_cmp(const void *UNUSED(fndata),
 			    const struct hashmap_entry *eptr,
 			    const struct hashmap_entry *entry_or_key,
 			    const void *key)
diff --git a/strmap.c b/strmap.c
index ee48635708..4e79734e4f 100644
--- a/strmap.c
+++ b/strmap.c
@@ -2,10 +2,10 @@
 #include "strmap.h"
 #include "mem-pool.h"
 
-int cmp_strmap_entry(const void *hashmap_cmp_fn_data,
+int cmp_strmap_entry(const void *UNUSED(hashmap_cmp_fn_data),
 		     const struct hashmap_entry *entry1,
 		     const struct hashmap_entry *entry2,
-		     const void *keydata)
+		     const void *UNUSED(keydata))
 {
 	const struct strmap_entry *e1, *e2;
 
diff --git a/sub-process.c b/sub-process.c
index cae56ae6b8..bd6a372a67 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -5,10 +5,10 @@
 #include "sigchain.h"
 #include "pkt-line.h"
 
-int cmd2process_cmp(const void *unused_cmp_data,
+int cmd2process_cmp(const void *UNUSED(cmp_data),
 		    const struct hashmap_entry *eptr,
 		    const struct hashmap_entry *entry_or_key,
-		    const void *unused_keydata)
+		    const void *UNUSED(keydata))
 {
 	const struct subprocess_entry *e1, *e2;
 
diff --git a/submodule-config.c b/submodule-config.c
index c2ac7e7bf3..d7a8ca0269 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -38,10 +38,10 @@ enum lookup_type {
 	lookup_path
 };
 
-static int config_path_cmp(const void *unused_cmp_data,
+static int config_path_cmp(const void *UNUSED(cmp_data),
 			   const struct hashmap_entry *eptr,
 			   const struct hashmap_entry *entry_or_key,
-			   const void *unused_keydata)
+			   const void *UNUSED(keydata))
 {
 	const struct submodule_entry *a, *b;
 
@@ -52,10 +52,10 @@ static int config_path_cmp(const void *unused_cmp_data,
 	       !oideq(&a->config->gitmodules_oid, &b->config->gitmodules_oid);
 }
 
-static int config_name_cmp(const void *unused_cmp_data,
+static int config_name_cmp(const void *UNUSED(cmp_data),
 			   const struct hashmap_entry *eptr,
 			   const struct hashmap_entry *entry_or_key,
-			   const void *unused_keydata)
+			   const void *UNUSED(keydata))
 {
 	const struct submodule_entry *a, *b;
 
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 09/11] mark unused read_tree_recursive() callback parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (7 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 08/11] hashmap: " Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 10/11] run-command: mark unused async " Jeff King
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

We pass a callback to read_tree_recursive(), but not every callback
needs every parameter. Let's mark the unused ones to satisfy
-Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 archive-tar.c      |  2 +-
 archive-zip.c      |  2 +-
 archive.c          |  3 ++-
 builtin/checkout.c |  2 +-
 builtin/log.c      |  7 ++++---
 builtin/ls-tree.c  | 13 ++++++++-----
 merge-recursive.c  |  2 +-
 7 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index 45e5d91407..0d66a1e0a8 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -421,7 +421,7 @@ static int git_tar_config(const char *var, const char *value, void *cb)
 	return tar_filter_config(var, value, cb);
 }
 
-static int write_tar_archive(const struct archiver *ar,
+static int write_tar_archive(const struct archiver *UNUSED(ar),
 			     struct archiver_args *args)
 {
 	int err = 0;
diff --git a/archive-zip.c b/archive-zip.c
index 854bceb018..d63782dc31 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -618,7 +618,7 @@ static int archive_zip_config(const char *var, const char *value,
 	return userdiff_config(var, value);
 }
 
-static int write_zip_archive(const struct archiver *ar,
+static int write_zip_archive(const struct archiver *UNUSED(ar),
 			     struct archiver_args *args)
 {
 	int err;
diff --git a/archive.c b/archive.c
index d5109abb89..8b165e935b 100644
--- a/archive.c
+++ b/archive.c
@@ -382,7 +382,8 @@ struct path_exists_context {
 	struct archiver_args *args;
 };
 
-static int reject_entry(const struct object_id *oid, struct strbuf *base,
+static int reject_entry(const struct object_id *UNUSED(oid),
+			struct strbuf *base,
 			const char *filename, unsigned mode,
 			void *context)
 {
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 713410ce2c..d18c8c886e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -125,7 +125,7 @@ static int post_checkout_hook(struct commit *old_commit, struct commit *new_comm
 }
 
 static int update_some(const struct object_id *oid, struct strbuf *base,
-		const char *pathname, unsigned mode, void *context)
+		       const char *pathname, unsigned mode, void *UNUSED(context))
 {
 	int len;
 	struct cache_entry *ce;
diff --git a/builtin/log.c b/builtin/log.c
index 9b937d59b8..79a2e4d0bb 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -645,9 +645,10 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev)
 	return 0;
 }
 
-static int show_tree_object(const struct object_id *oid,
-		struct strbuf *base,
-		const char *pathname, unsigned mode, void *context)
+static int show_tree_object(const struct object_id *UNUSED(oid),
+			    struct strbuf *UNUSED(base),
+			    const char *pathname, unsigned mode,
+			    void *context)
 {
 	FILE *file = context;
 	fprintf(file, "%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index e279be8bb6..48df337605 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -142,7 +142,7 @@ static int show_recursive(const char *base, size_t baselen, const char *pathname
 }
 
 static int show_tree_fmt(const struct object_id *oid, struct strbuf *base,
-			 const char *pathname, unsigned mode, void *context)
+			 const char *pathname, unsigned mode, void *UNUSED(context))
 {
 	size_t baselen;
 	int recurse = 0;
@@ -213,7 +213,7 @@ static void show_tree_common_default_long(struct strbuf *base,
 
 static int show_tree_default(const struct object_id *oid, struct strbuf *base,
 			     const char *pathname, unsigned mode,
-			     void *context)
+			     void *UNUSED(context))
 {
 	int early;
 	int recurse;
@@ -230,7 +230,8 @@ static int show_tree_default(const struct object_id *oid, struct strbuf *base,
 }
 
 static int show_tree_long(const struct object_id *oid, struct strbuf *base,
-			  const char *pathname, unsigned mode, void *context)
+			  const char *pathname, unsigned mode,
+			  void *UNUSED(context))
 {
 	int early;
 	int recurse;
@@ -259,7 +260,8 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 }
 
 static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
-			       const char *pathname, unsigned mode, void *context)
+			       const char *pathname, unsigned mode,
+			       void *UNUSED(context))
 {
 	int early;
 	int recurse;
@@ -279,7 +281,8 @@ static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
 }
 
 static int show_tree_object(const struct object_id *oid, struct strbuf *base,
-			    const char *pathname, unsigned mode, void *context)
+			    const char *pathname, unsigned mode,
+			    void *UNUSED(context))
 {
 	int early;
 	int recurse;
diff --git a/merge-recursive.c b/merge-recursive.c
index 775ebe2182..08c1c36d33 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -456,7 +456,7 @@ static void unpack_trees_finish(struct merge_options *opt)
 	clear_unpack_trees_porcelain(&opt->priv->unpack_opts);
 }
 
-static int save_files_dirs(const struct object_id *oid,
+static int save_files_dirs(const struct object_id *UNUSED(oid),
 			   struct strbuf *base, const char *path,
 			   unsigned int mode, void *context)
 {
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 10/11] run-command: mark unused async callback parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (8 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 09/11] mark unused read_tree_recursive() " Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 10:08 ` [PATCH 11/11] is_path_owned_by_current_uid(): mark "report" parameter as unused Jeff King
  2022-08-19 13:58 ` [PATCH 0/11] annotating unused function parameters Ævar Arnfjörð Bjarmason
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

The start_async(), etc, functions need a "proc" callback that conforms
to a particular interface. Not every callback needs every parameter
(e.g., the caller might not even ask to open an input descriptor, in
which case there is no point in the callback looking at it). Let's mark
these for -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/receive-pack.c | 2 +-
 convert.c              | 2 +-
 fetch-pack.c           | 2 +-
 send-pack.c            | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index afd36c9c53..6882d526e6 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -465,7 +465,7 @@ static void rp_error(const char *err, ...)
 	va_end(params);
 }
 
-static int copy_to_sideband(int in, int out, void *arg)
+static int copy_to_sideband(int in, int UNUSED(out), void *UNUSED(arg))
 {
 	char data[128];
 	int keepalive_active = 0;
diff --git a/convert.c b/convert.c
index b31a25b536..25d89fa83b 100644
--- a/convert.c
+++ b/convert.c
@@ -619,7 +619,7 @@ struct filter_params {
 	const char *path;
 };
 
-static int filter_buffer_or_fd(int in, int out, void *data)
+static int filter_buffer_or_fd(int UNUSED(in), int out, void *data)
 {
 	/*
 	 * Spawn cmd and feed the buffer contents through its stdin.
diff --git a/fetch-pack.c b/fetch-pack.c
index bda9d0f433..9f2933e868 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -823,7 +823,7 @@ static int everything_local(struct fetch_pack_args *args,
 	return retval;
 }
 
-static int sideband_demux(int in, int out, void *data)
+static int sideband_demux(int UNUSED(in), int out, void *data)
 {
 	int *xd = data;
 	int ret;
diff --git a/send-pack.c b/send-pack.c
index 662f7c2aeb..7e99c64e6b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -266,7 +266,7 @@ static int receive_status(struct packet_reader *reader, struct ref *refs)
 	return ret;
 }
 
-static int sideband_demux(int in, int out, void *data)
+static int sideband_demux(int UNUSED(in), int out, void *data)
 {
 	int *fd = data, ret;
 	if (async_with_fork())
-- 
2.37.2.928.g0821088f4a


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 11/11] is_path_owned_by_current_uid(): mark "report" parameter as unused
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (9 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 10/11] run-command: mark unused async " Jeff King
@ 2022-08-19 10:08 ` Jeff King
  2022-08-19 13:58 ` [PATCH 0/11] annotating unused function parameters Ævar Arnfjörð Bjarmason
  11 siblings, 0 replies; 22+ messages in thread
From: Jeff King @ 2022-08-19 10:08 UTC (permalink / raw)
  To: git

In the non-Windows version of this function, we never have any errors to
report, and thus the "report" parameter is unused. But we can't drop it,
because we have to maintain function call compatibility with the version
in compat/mingw.h, which does use this parameter.

Note that there's an extra level of indirection here; the common
function is actually is_path_owned_by_current_user, which is a macro
pointing to "by_current_uid" or "by_current_sid", depending on the
platform. So an alternative here is to eat the unused parameter in the
macro, since -Wunused-parameter doesn't complain about macros. But I
think the UNUSED() annotation is less obfuscated for somebody reading
the code later.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-compat-util.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 12239fedf7..a9690126bb 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -498,7 +498,8 @@ static inline void extract_id_from_env(const char *env, uid_t *id)
 	}
 }
 
-static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report)
+static inline int is_path_owned_by_current_uid(const char *path,
+					       struct strbuf *UNUSED(report))
 {
 	struct stat st;
 	uid_t euid;
-- 
2.37.2.928.g0821088f4a

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
                   ` (10 preceding siblings ...)
  2022-08-19 10:08 ` [PATCH 11/11] is_path_owned_by_current_uid(): mark "report" parameter as unused Jeff King
@ 2022-08-19 13:58 ` Ævar Arnfjörð Bjarmason
  2022-08-19 18:59   ` Derrick Stolee
  11 siblings, 1 reply; 22+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-19 13:58 UTC (permalink / raw)
  To: Jeff King; +Cc: git


On Fri, Aug 19 2022, Jeff King wrote:

> I've been carrying a bunch of patches (for almost 4 years now!) that get
> the code base compiling cleanly with -Wunused-parameter. This is a
> useful warning in my opinion; it found real bugs[1] when applied to the
> whole code base. So it would be nice to be able to turn it on all the
> time and get the same protection going forward.
> [...]
> And of course the most important question is: do we like this direction
> overall. This mass-annotation is a one-time pain. Going forward, the
> only work would be requiring people to annotate new functions they add
> (which again, is mostly going to be callbacks). IMHO it's worth it. In
> addition to possibly finding errors, I think the annotations serve as an
> extra clue for people reading the code about what the author intended.

I've known you've had this out-of-tree for a while, and really like that
it's on the path to getting integrated.

But I have a hang-up about it, which is that I though __attribute__
(unused) didn't work like *that*.

What it means (and maybe only I find this counter-intuitive) is "trust
me, this is unused, but don't check!", furthermore it causes the
compiler to completely ignore the variable for the purposes of *all*
warnings, not just the unused one.

I may still be missing something, but I wonder if this squashed in
wouldn't be much better:
	
	diff --git a/git-compat-util.h b/git-compat-util.h
	index a9690126bb0..e02e2fc3f6d 100644
	--- a/git-compat-util.h
	+++ b/git-compat-util.h
	@@ -190,9 +190,9 @@ struct strbuf;
	 #define _SGI_SOURCE 1
	 
	 #if defined(__GNUC__)
	-#define UNUSED(var) UNUSED_##var __attribute__((unused))
	+#define UNUSED(var) var __attribute__((deprecated ("not 'deprecated', but expected not to be used!")))
	 #else
	-#define UNUSED(var) UNUSED_##var
	+#define UNUSED(var) var
	 #endif
	 
	 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */

I.e. it's a bit counter-intuitive to mark these as "deprecated", but you
can add a custom message (with both GCC and clang). Improvements to the
message welcome.

Now as in this series we stay silent if the variable is not used, but we
*don't* stay silent if an UNUSED(var) is actually used, that'll now be
an error:
	
	xdiff/xdiffi.c: In function ‘xdl_call_hunk_func’:
	xdiff/xdiffi.c:981:9: error: ‘xe’ is deprecated: not 'deprecated', but expected not to be used! [-Werror=deprecated-declarations]
	  981 |         fprintf(stderr, "%p", (void*)xe);
	      |         ^~~~~~~

This also means that you don't need to rename the variable just to avoid
"accidental" use, which also has the benefit of not tripping up the
variable typo detection.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-19 13:58 ` [PATCH 0/11] annotating unused function parameters Ævar Arnfjörð Bjarmason
@ 2022-08-19 18:59   ` Derrick Stolee
  2022-08-19 20:58     ` Ævar Arnfjörð Bjarmason
  0 siblings, 1 reply; 22+ messages in thread
From: Derrick Stolee @ 2022-08-19 18:59 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason, Jeff King; +Cc: git

On 8/19/2022 9:58 AM, Ævar Arnfjörð Bjarmason wrote:
> 
> On Fri, Aug 19 2022, Jeff King wrote:
> 
>> I've been carrying a bunch of patches (for almost 4 years now!) that get
>> the code base compiling cleanly with -Wunused-parameter. This is a
>> useful warning in my opinion; it found real bugs[1] when applied to the
>> whole code base. So it would be nice to be able to turn it on all the
>> time and get the same protection going forward.
>> [...]
>> And of course the most important question is: do we like this direction
>> overall. This mass-annotation is a one-time pain. Going forward, the
>> only work would be requiring people to annotate new functions they add
>> (which again, is mostly going to be callbacks). IMHO it's worth it. In
>> addition to possibly finding errors, I think the annotations serve as an
>> extra clue for people reading the code about what the author intended.
> 
> I've known you've had this out-of-tree for a while, and really like that
> it's on the path to getting integrated.
> 
> But I have a hang-up about it, which is that I though __attribute__
> (unused) didn't work like *that*.
> 
> What it means (and maybe only I find this counter-intuitive) is "trust
> me, this is unused, but don't check!", furthermore it causes the
> compiler to completely ignore the variable for the purposes of *all*
> warnings, not just the unused one.

That's not the reason for the attribute at all. It's supposed to say "I
know this is unused, but I still need it to be in the parameter list for
other reasons. Don't create a warning for this case."

Interpreting it the way you are means "don't do the analysis. Just throw a
warning." which doesn't make any sense.

> I may still be missing something, but I wonder if this squashed in
> wouldn't be much better:
> 	
> 	diff --git a/git-compat-util.h b/git-compat-util.h
> 	index a9690126bb0..e02e2fc3f6d 100644
> 	--- a/git-compat-util.h
> 	+++ b/git-compat-util.h
> 	@@ -190,9 +190,9 @@ struct strbuf;
> 	 #define _SGI_SOURCE 1
> 	 
> 	 #if defined(__GNUC__)
> 	-#define UNUSED(var) UNUSED_##var __attribute__((unused))
> 	+#define UNUSED(var) var __attribute__((deprecated ("not 'deprecated', but expected not to be used!")))
> 	 #else
> 	-#define UNUSED(var) UNUSED_##var
> 	+#define UNUSED(var) var
> 	 #endif

Does the deprecated attribute imply unused? Or at the very least, does it
avoid the -Wunused-parameter warnings?

It might be helpful to _also_ have a deprecated annotation so we know to
remove the UNUSED macro if a parameter starts being used again. The
existing macro changes the variable name so we would get compiler errors
if we started using it, but we could have a better message indicating
exactly why things are not working.

So in that sense, you are onto something. Should we use both attributes?

At the very least, the warning message you recommend in the 'deprecated'
attribute could be more direct about what we expect.
	 
	 #if defined(__GNUC__)
	-#define UNUSED(var) UNUSED_##var __attribute__((unused))
	+#define UNUSED(var) var __attribute__((unused)) \
				 __attribute__((deprecated ("remove UNUSED macro before using")))
	 #else
	-#define UNUSED(var) UNUSED_##var
	+#define UNUSED(var) var
	 #endif
	 
	 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-19 18:59   ` Derrick Stolee
@ 2022-08-19 20:58     ` Ævar Arnfjörð Bjarmason
  2022-08-20  9:46       ` Jeff King
  0 siblings, 1 reply; 22+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-19 20:58 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: Jeff King, git


On Fri, Aug 19 2022, Derrick Stolee wrote:

> On 8/19/2022 9:58 AM, Ævar Arnfjörð Bjarmason wrote:
>> 
>> On Fri, Aug 19 2022, Jeff King wrote:
>> 
>>> I've been carrying a bunch of patches (for almost 4 years now!) that get
>>> the code base compiling cleanly with -Wunused-parameter. This is a
>>> useful warning in my opinion; it found real bugs[1] when applied to the
>>> whole code base. So it would be nice to be able to turn it on all the
>>> time and get the same protection going forward.
>>> [...]
>>> And of course the most important question is: do we like this direction
>>> overall. This mass-annotation is a one-time pain. Going forward, the
>>> only work would be requiring people to annotate new functions they add
>>> (which again, is mostly going to be callbacks). IMHO it's worth it. In
>>> addition to possibly finding errors, I think the annotations serve as an
>>> extra clue for people reading the code about what the author intended.
>> 
>> I've known you've had this out-of-tree for a while, and really like that
>> it's on the path to getting integrated.
>> 
>> But I have a hang-up about it, which is that I though __attribute__
>> (unused) didn't work like *that*.
>> 
>> What it means (and maybe only I find this counter-intuitive) is "trust
>> me, this is unused, but don't check!", furthermore it causes the
>> compiler to completely ignore the variable for the purposes of *all*
>> warnings, not just the unused one.
>
> That's not the reason for the attribute at all. It's supposed to say "I
> know this is unused, but I still need it to be in the parameter list for
> other reasons. Don't create a warning for this case."
>
> Interpreting it the way you are means "don't do the analysis. Just throw a
> warning." which doesn't make any sense.
>
>> I may still be missing something, but I wonder if this squashed in
>> wouldn't be much better:
>> 	
>> 	diff --git a/git-compat-util.h b/git-compat-util.h
>> 	index a9690126bb0..e02e2fc3f6d 100644
>> 	--- a/git-compat-util.h
>> 	+++ b/git-compat-util.h
>> 	@@ -190,9 +190,9 @@ struct strbuf;
>> 	 #define _SGI_SOURCE 1
>> 	 
>> 	 #if defined(__GNUC__)
>> 	-#define UNUSED(var) UNUSED_##var __attribute__((unused))
>> 	+#define UNUSED(var) var __attribute__((deprecated ("not 'deprecated', but expected not to be used!")))
>> 	 #else
>> 	-#define UNUSED(var) UNUSED_##var
>> 	+#define UNUSED(var) var
>> 	 #endif
>
> Does the deprecated attribute imply unused? Or at the very least, does it
> avoid the -Wunused-parameter warnings?
>
> It might be helpful to _also_ have a deprecated annotation so we know to
> remove the UNUSED macro if a parameter starts being used again. The
> existing macro changes the variable name so we would get compiler errors
> if we started using it, but we could have a better message indicating
> exactly why things are not working.
>
> So in that sense, you are onto something. Should we use both attributes?
>
> At the very least, the warning message you recommend in the 'deprecated'
> attribute could be more direct about what we expect.
> 	 
> 	 #if defined(__GNUC__)
> 	-#define UNUSED(var) UNUSED_##var __attribute__((unused))
> 	+#define UNUSED(var) var __attribute__((unused)) \
> 				 __attribute__((deprecated ("remove UNUSED macro before using")))
> 	 #else
> 	-#define UNUSED(var) UNUSED_##var
> 	+#define UNUSED(var) var
> 	 #endif
> 	 
> 	 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */

Yes, I spoke too soon, sorry. We still need ((unused)). FWIW the below
on top of master and doing:

	 make bisect.o CFLAGS=-Wunused-parameter

Does the right thing for me, and then warns if you uncomment that "//"
commented fprintf().

The reason I was confused is that -Wunused-parameter is *not* needed
with ((deprecated)) to error if the variable is used, but it *is* needed
to hide it from -Wunused-parameter if you're trying to find the next
thing to mark as "UNUSED" (or to refactor away).

I think the below version of it also shows that you don't need to pass
the variable name to the macro. If the only reason for that was to avoid
accidental use it seems like the ((deprecated)) attribute should cover
it.

I'd prefer it if we could avoid the funny syntax, it's highlighted a bit
weirdly in my editor, and I suspect I'm not the only one, but mainly
having the extra compiler-enforced sanity checking of checking if it's
accidentally used, without renaming the variable, which seems like a bit
of a hack.

diff --git a/bisect.c b/bisect.c
index 38b3891f3a6..fd581b85a72 100644
--- a/bisect.c
+++ b/bisect.c
@@ -441,7 +441,7 @@ void find_bisection(struct commit_list **commit_list, int *reaches,
 }
 
 static int register_ref(const char *refname, const struct object_id *oid,
-			int flags, void *cb_data)
+			int flags UNUSED, void *cb_data UNUSED)
 {
 	struct strbuf good_prefix = STRBUF_INIT;
 	strbuf_addstr(&good_prefix, term_good);
@@ -1160,8 +1160,9 @@ int estimate_bisect_steps(int all)
 	return (e < 3 * x) ? n : n - 1;
 }
 
-static int mark_for_removal(const char *refname, const struct object_id *oid,
-			    int flag, void *cb_data)
+static int mark_for_removal(const char *refname,
+			    const struct object_id *oid UNUSED,
+			    int flag UNUSED, void *cb_data)
 {
 	struct string_list *refs = cb_data;
 	char *ref = xstrfmt("refs/bisect%s", refname);
diff --git a/git-compat-util.h b/git-compat-util.h
index 36a25ae252f..7f7395fc9f7 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -189,6 +189,12 @@ struct strbuf;
 #define _NETBSD_SOURCE 1
 #define _SGI_SOURCE 1
 
+#if defined(__GNUC__)
+#define UNUSED __attribute__((unused)) __attribute__((deprecated ("marked with UNUSED")))
+#else
+#define UNUSED
+#endif
+
 #if defined(WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
 # if !defined(_WIN32_WINNT)
 #  define _WIN32_WINNT 0x0600
@@ -302,7 +308,8 @@ typedef unsigned long uintptr_t;
 #ifdef PRECOMPOSE_UNICODE
 #include "compat/precompose_utf8.h"
 #else
-static inline const char *precompose_argv_prefix(int argc, const char **argv, const char *prefix)
+static inline const char *precompose_argv_prefix(int argc UNUSED, const char **argv UNUSED,
+						 const char *prefix)
 {
 	return prefix;
 }
@@ -397,7 +404,8 @@ typedef uintmax_t timestamp_t;
 #endif
 
 #ifndef platform_core_config
-static inline int noop_core_config(const char *var, const char *value, void *cb)
+static inline int noop_core_config(const char *var UNUSED, const char *value UNUSED,
+				   void *cb UNUSED)
 {
 	return 0;
 }
@@ -410,7 +418,7 @@ int lstat_cache_aware_rmdir(const char *path);
 #endif
 
 #ifndef has_dos_drive_prefix
-static inline int git_has_dos_drive_prefix(const char *path)
+static inline int git_has_dos_drive_prefix(const char *path UNUSED)
 {
 	return 0;
 }
@@ -418,7 +426,7 @@ static inline int git_has_dos_drive_prefix(const char *path)
 #endif
 
 #ifndef skip_dos_drive_prefix
-static inline int git_skip_dos_drive_prefix(char **path)
+static inline int git_skip_dos_drive_prefix(char **path UNUSED)
 {
 	return 0;
 }
@@ -490,11 +498,13 @@ static inline void extract_id_from_env(const char *env, uid_t *id)
 	}
 }
 
-static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report)
+static inline int is_path_owned_by_current_uid(const char *path, struct strbuf *report UNUSED)
 {
 	struct stat st;
 	uid_t euid;
 
+	//fprintf(stderr, "%p", (void*)report);
+
 	if (lstat(path, &st))
 		return 0;
 
diff --git a/object-store.h b/object-store.h
index 5222ee54600..218ffe73604 100644
--- a/object-store.h
+++ b/object-store.h
@@ -141,7 +141,7 @@ struct packed_git {
 
 struct multi_pack_index;
 
-static inline int pack_map_entry_cmp(const void *unused_cmp_data,
+static inline int pack_map_entry_cmp(const void *unused_cmp_data UNUSED,
 				     const struct hashmap_entry *entry,
 				     const struct hashmap_entry *entry2,
 				     const void *keydata)


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-19 20:58     ` Ævar Arnfjörð Bjarmason
@ 2022-08-20  9:46       ` Jeff King
  2022-08-20 21:21         ` Junio C Hamano
                           ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Jeff King @ 2022-08-20  9:46 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Derrick Stolee, git

On Fri, Aug 19, 2022 at 10:58:08PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Yes, I spoke too soon, sorry. We still need ((unused)). FWIW the below
> on top of master and doing:

Right. Using ((deprecated)) is really just a substitute for the variable
renaming part.

And I agree it works as advertised, though I think I prefer the
variable-renaming version.

One, it feels like we're abusing the deprecated attribute here. The
confusion in the compiler output I'm OK with, because we get a chance to
put our own message there (so I agree the output is actually better than
with my patch). But from time to time I've had to build with
-Wno-deprecated-declarations to get around _actual_ deprecated warnings
(e.g., compiling with OPENSSL_SHA1=Yes). And doing so would be cutting
out half the protection of UNUSED() in that case.

Likewise, one thing I like about the renaming is that it fails
compilation regardless of -Werror. So it will be caught in any compile,
no matter what. And I do automatically compile without DEVELOPER=1 when
on a detached HEAD, because historical commits often trigger warnings.
Go back far enough and OPENSSL_SHA1 was the default, which generates
lots of warnings these days. :)

And finally, I actually prefer the parentheses of:

  static int register_ref(const char *refname, const struct object_id *oid,
			  int UNUSED(flags), void *UNUSED(cb_data))

It visually binds the attribute more tightly to the variable name, like
how we sometimes write unused_flags manually in existing code. When
there are a lot of variables to mark in a function (and some callbacks
really do have a lot), that makes it easier to see what's going on. To
me, anyway. I recognize that it's a matter of taste.

Technically this last thing is orthogonal to using the deprecated
attribute. It could still be used with the parenthesized form, but the
error messages gcc generates are horrendous then (it repeats the warning
several times due to the macro).

So I dunno. These are all matters of opinion, and if it was just my
patches, I'd say my taste wins. But all of us are going to have to write
these annotations at some time or another when we add callbacks, etc. So
we should at the very least pick a syntax the majority prefers. :)

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-20  9:46       ` Jeff King
@ 2022-08-20 21:21         ` Junio C Hamano
  2022-08-22 14:14         ` Derrick Stolee
  2022-08-25 11:00         ` Ævar Arnfjörð Bjarmason
  2 siblings, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2022-08-20 21:21 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, Derrick Stolee, git

Jeff King <peff@peff.net> writes:

> Likewise, one thing I like about the renaming is that it fails
> compilation regardless of -Werror.

Yes, I like that aspect of the macro very much.

> And finally, I actually prefer the parentheses of:
>
>   static int register_ref(const char *refname, const struct object_id *oid,
> 			  int UNUSED(flags), void *UNUSED(cb_data))

That, too.

> So I dunno. These are all matters of opinion, and if it was just my
> patches, I'd say my taste wins. But all of us are going to have to write
> these annotations at some time or another when we add callbacks, etc. So
> we should at the very least pick a syntax the majority prefers. :)

Well, we can teach others a good taste ;-)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-20  9:46       ` Jeff King
  2022-08-20 21:21         ` Junio C Hamano
@ 2022-08-22 14:14         ` Derrick Stolee
  2022-08-25 11:00         ` Ævar Arnfjörð Bjarmason
  2 siblings, 0 replies; 22+ messages in thread
From: Derrick Stolee @ 2022-08-22 14:14 UTC (permalink / raw)
  To: Jeff King, Ævar Arnfjörð Bjarmason; +Cc: git

On 8/20/2022 5:46 AM, Jeff King wrote:
> On Fri, Aug 19, 2022 at 10:58:08PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
>> Yes, I spoke too soon, sorry. We still need ((unused)). FWIW the below
>> on top of master and doing:
> 
> Right. Using ((deprecated)) is really just a substitute for the variable
> renaming part.
> 
> And I agree it works as advertised, though I think I prefer the
> variable-renaming version.
> 
> One, it feels like we're abusing the deprecated attribute here. The
> confusion in the compiler output I'm OK with, because we get a chance to
> put our own message there (so I agree the output is actually better than
> with my patch). But from time to time I've had to build with
> -Wno-deprecated-declarations to get around _actual_ deprecated warnings
> (e.g., compiling with OPENSSL_SHA1=Yes). And doing so would be cutting
> out half the protection of UNUSED() in that case.

The fact that we can't turn on -Wno-deprecated-declarations is enough
to convince me that we need to use the variable renaming trick.

Thanks,
-Stolee


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-20  9:46       ` Jeff King
  2022-08-20 21:21         ` Junio C Hamano
  2022-08-22 14:14         ` Derrick Stolee
@ 2022-08-25 11:00         ` Ævar Arnfjörð Bjarmason
  2022-08-26  7:10           ` Jeff King
  2 siblings, 1 reply; 22+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-08-25 11:00 UTC (permalink / raw)
  To: Jeff King; +Cc: Derrick Stolee, git


On Sat, Aug 20 2022, Jeff King wrote:

> On Fri, Aug 19, 2022 at 10:58:08PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Yes, I spoke too soon, sorry. We still need ((unused)). FWIW the below
>> on top of master and doing:
>
> Right. Using ((deprecated)) is really just a substitute for the variable
> renaming part.
>
> And I agree it works as advertised, though I think I prefer the
> variable-renaming version.
>
> One, it feels like we're abusing the deprecated attribute here. The

Definitely, but structurally it seems like a better pick. I.e. isn't the
only problem with it the "deprecated" and its interaction with
-Wno-deprecated.

If the exact same feature existed as a "insert-custom-warning", which
would work exactly "deprecated" without the default warning "prefix"
would you think this would fit perfectly?

> ...
> -Wno-deprecated-declarations to get around _actual_ deprecated warnings
> (e.g., compiling with OPENSSL_SHA1=Yes). And doing so would be cutting
> out half the protection of UNUSED() in that case.

This is mildly annoying, but I don't really think it's a practical
issue. We're talking about running this without
-Wno-deprecated-declarations in CI, and by default.

For unused parameters it's enough that we're catching them somewhere, or
in common compilation settings, we don't need to catch them
*everywhere*, do we?

IOW is anyone writing patches where they're testing with
-Wno-deprecated-declarations *and* adding unused parameters *and* won't
test without -Wno-deprecated-declarations before submitting them, *and*
nobody else will catch it?

> Likewise, one thing I like about the renaming is that it fails
> compilation regardless of -Werror. So it will be caught in any compile,
> no matter what. And I do automatically compile without DEVELOPER=1 when
> on a detached HEAD, because historical commits often trigger warnings.
> Go back far enough and OPENSSL_SHA1 was the default, which generates
> lots of warnings these days. :)

*nod*, I think this also goes the other way. It's nice to be able to use
DEVOPTS=no-error to "get past" various minor issues. I consider an
unused parameter as being a minor issue. E.g. when ad-hoc cherry-picking
something to test on an older version it can be annoying to have to make
larger changes when a DEVOPTS=no-error would do.

> And finally, I actually prefer the parentheses of:
>
>   static int register_ref(const char *refname, const struct object_id *oid,
> 			  int UNUSED(flags), void *UNUSED(cb_data))

...and now to the real reason for the follow-up. You/Junio were CC'd,
but this is breaking coccinelle, see:
https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/

So, bikeshedding-wise I don't care to argue the point. But between odd
syntax highlighting and now one analysis tool barfing on it it's a bit
more than a bikeshed :)

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-25 11:00         ` Ævar Arnfjörð Bjarmason
@ 2022-08-26  7:10           ` Jeff King
  2022-08-26 13:08             ` Phillip Wood
  2022-08-26 16:37             ` Junio C Hamano
  0 siblings, 2 replies; 22+ messages in thread
From: Jeff King @ 2022-08-26  7:10 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: Derrick Stolee, git

On Thu, Aug 25, 2022 at 01:00:19PM +0200, Ævar Arnfjörð Bjarmason wrote:

> > One, it feels like we're abusing the deprecated attribute here. The
> 
> Definitely, but structurally it seems like a better pick. I.e. isn't the
> only problem with it the "deprecated" and its interaction with
> -Wno-deprecated.
> 
> If the exact same feature existed as a "insert-custom-warning", which
> would work exactly "deprecated" without the default warning "prefix"
> would you think this would fit perfectly?

Yeah, I agree that would remove my complaint about overloading the
meaning. I don't think that exists, though.

> This is mildly annoying, but I don't really think it's a practical
> issue. We're talking about running this without
> -Wno-deprecated-declarations in CI, and by default.
> 
> For unused parameters it's enough that we're catching them somewhere, or
> in common compilation settings, we don't need to catch them
> *everywhere*, do we?

No, but the farther away you go from the edit-compile-run cycle, the
more painful warnings become. Catching them immediately and fully has
real value, as it means the cost of correcting them is lower. So all
things being equal, I think we should prefer universal solutions when
they're available (and for example compiler errors over say, coccinelle
or other analysis tools).

(And yes, I know all things sadly aren't equal; see below...)

> IOW is anyone writing patches where they're testing with
> -Wno-deprecated-declarations *and* adding unused parameters *and* won't
> test without -Wno-deprecated-declarations before submitting them, *and*
> nobody else will catch it?

Probably not. I don't actually build with -Wno-deprecated-declarations
routinely. But my fear is that some platform may be stuck there for a
while (because an overzealous libc marks something). But that's kind of
hypothetical, so we may have to just accept it and cross that bridge if
we come to it.

> > And finally, I actually prefer the parentheses of:
> >
> >   static int register_ref(const char *refname, const struct object_id *oid,
> > 			  int UNUSED(flags), void *UNUSED(cb_data))
> 
> ...and now to the real reason for the follow-up. You/Junio were CC'd,
> but this is breaking coccinelle, see:
> https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/

Ugh. Yeah, that is really unfortunate. I much prefer the parenthesized
syntax, but if we can't find a way to unconfuse third-party parsing,
then switching is probably the least-bad solution.

-Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-26  7:10           ` Jeff King
@ 2022-08-26 13:08             ` Phillip Wood
  2022-08-26 16:37             ` Junio C Hamano
  1 sibling, 0 replies; 22+ messages in thread
From: Phillip Wood @ 2022-08-26 13:08 UTC (permalink / raw)
  To: Jeff King
  Cc: Ævar Arnfjörð Bjarmason, Derrick Stolee, Git Mailing List

Hi Peff

On Fri, 26 Aug 2022 at 08:33, Jeff King <peff@peff.net> wrote:
>
> On Thu, Aug 25, 2022 at 01:00:19PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> > > One, it feels like we're abusing the deprecated attribute here. The
> >
> > Definitely, but structurally it seems like a better pick. I.e. isn't the
> > only problem with it the "deprecated" and its interaction with
> > -Wno-deprecated.
> > This is mildly annoying, but I don't really think it's a practical
> > issue. We're talking about running this without
> > -Wno-deprecated-declarations in CI, and by default.
> >
> > For unused parameters it's enough that we're catching them somewhere, or
> > in common compilation settings, we don't need to catch them
> > *everywhere*, do we?
>
> No, but the farther away you go from the edit-compile-run cycle, the
> more painful warnings become. Catching them immediately and fully has
> real value, as it means the cost of correcting them is lower. So all
> things being equal, I think we should prefer universal solutions when
> they're available (and for example compiler errors over say, coccinelle
> or other analysis tools).

That's a good point, one of the nice things about your macro was that
all compilers detected when UNUSED() parameters were in fact used. In
comparison abusing the deprecated attribute is uglier and less
practical.

> (And yes, I know all things sadly aren't equal; see below...)

It might be worth reporting this issue on the coccinelle mailing list
to see if anyone is interested in fixing it. If it gets fixed we're
left with the problem of having  to build it for our ci but that
shouldn't be insurmountable.

Best Wishes

Phillip

> > IOW is anyone writing patches where they're testing with
> > -Wno-deprecated-declarations *and* adding unused parameters *and* won't
> > test without -Wno-deprecated-declarations before submitting them, *and*
> > nobody else will catch it?
>
> Probably not. I don't actually build with -Wno-deprecated-declarations
> routinely. But my fear is that some platform may be stuck there for a
> while (because an overzealous libc marks something). But that's kind of
> hypothetical, so we may have to just accept it and cross that bridge if
> we come to it.
>
> > > And finally, I actually prefer the parentheses of:
> > >
> > >   static int register_ref(const char *refname, const struct object_id *oid,
> > >                       int UNUSED(flags), void *UNUSED(cb_data))
> >
> > ...and now to the real reason for the follow-up. You/Junio were CC'd,
> > but this is breaking coccinelle, see:
> > https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/
>
> Ugh. Yeah, that is really unfortunate. I much prefer the parenthesized
> syntax, but if we can't find a way to unconfuse third-party parsing,
> then switching is probably the least-bad solution.
>
> -Peff

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/11] annotating unused function parameters
  2022-08-26  7:10           ` Jeff King
  2022-08-26 13:08             ` Phillip Wood
@ 2022-08-26 16:37             ` Junio C Hamano
  1 sibling, 0 replies; 22+ messages in thread
From: Junio C Hamano @ 2022-08-26 16:37 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, Derrick Stolee, git

Jeff King <peff@peff.net> writes:

> No, but the farther away you go from the edit-compile-run cycle, the
> more painful warnings become. Catching them immediately and fully has
> real value, as it means the cost of correcting them is lower. So all
> things being equal, I think we should prefer universal solutions when
> they're available (and for example compiler errors over say, coccinelle
> or other analysis tools).

Thanks for saying it so succinctly.  Making compiler errors less
useful only to please Coccinelle is putting our priority wrong.

> Ugh. Yeah, that is really unfortunate. I much prefer the parenthesized
> syntax, but if we can't find a way to unconfuse third-party parsing,
> then switching is probably the least-bad solution.

Or have third-party fix the parsing ;-)  Until then, perhaps we have
to live with a suboptimal syntax.

Thanks.

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2022-08-26 16:37 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-19 10:07 [PATCH 0/11] annotating unused function parameters Jeff King
2022-08-19 10:08 ` [PATCH 01/11] git-compat-util: add UNUSED macro Jeff King
2022-08-19 10:08 ` [PATCH 02/11] refs: mark unused each_ref_fn parameters Jeff King
2022-08-19 10:08 ` [PATCH 03/11] refs: mark unused reflog callback parameters Jeff King
2022-08-19 10:08 ` [PATCH 04/11] refs: mark unused virtual method parameters Jeff King
2022-08-19 10:08 ` [PATCH 05/11] transport: mark bundle transport_options as unused Jeff King
2022-08-19 10:08 ` [PATCH 06/11] streaming: mark unused virtual method parameters Jeff King
2022-08-19 10:08 ` [PATCH 07/11] config: mark unused callback parameters Jeff King
2022-08-19 10:08 ` [PATCH 08/11] hashmap: " Jeff King
2022-08-19 10:08 ` [PATCH 09/11] mark unused read_tree_recursive() " Jeff King
2022-08-19 10:08 ` [PATCH 10/11] run-command: mark unused async " Jeff King
2022-08-19 10:08 ` [PATCH 11/11] is_path_owned_by_current_uid(): mark "report" parameter as unused Jeff King
2022-08-19 13:58 ` [PATCH 0/11] annotating unused function parameters Ævar Arnfjörð Bjarmason
2022-08-19 18:59   ` Derrick Stolee
2022-08-19 20:58     ` Ævar Arnfjörð Bjarmason
2022-08-20  9:46       ` Jeff King
2022-08-20 21:21         ` Junio C Hamano
2022-08-22 14:14         ` Derrick Stolee
2022-08-25 11:00         ` Ævar Arnfjörð Bjarmason
2022-08-26  7:10           ` Jeff King
2022-08-26 13:08             ` Phillip Wood
2022-08-26 16:37             ` Junio C Hamano

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