threads / patch / 60161

patch, 22 partsYAUPS: Yet Another Unused Parameter Series

Subject: [PATCH 0/22] YAUPS: Yet Another Unused Parameter Series

## tl;dr

59 messages between Aug 28, 2023 and Aug 30, 2023. Diffs are folded; open one to read it.

replies: 58people: 4as markdown or json

Jeff King· Aug 28, 2023, 21:46 UTC · lore

I'm back with another exciting installment of -Wunused-parameter warning fixes. Most of these are pretty boring and obvious; the first two are the most interesting in terms of rationale.

I promise we're closing in on the finish line here. I only have about 20 patches left after this, at which point we should be able to turn on the warning by default for developer builds.

  [01/22]: sequencer: use repository parameter in short_commit_name()
  [02/22]: sequencer: mark repository argument as unused
  [03/22]: ref-filter: mark unused parameters in parser callbacks
  [04/22]: pack-bitmap: mark unused parameters in show_object callback
  [05/22]: worktree: mark unused parameters in each_ref_fn callback
  [06/22]: commit-graph: mark unused data parameters in generation callbacks
  [07/22]: ls-tree: mark unused parameter in callback
  [08/22]: stash: mark unused parameter in diff callback
  [09/22]: trace2: mark unused us_elapsed_absolute parameters
  [10/22]: trace2: mark unused config callback parameter
  [11/22]: test-trace2: mark unused argv/argc parameters
  [12/22]: grep: mark unused parameter in output function
  [13/22]: add-interactive: mark unused callback parameters
  [14/22]: negotiator/noop: mark unused callback parameters
  [15/22]: worktree: mark unused parameters in noop repair callback
  [16/22]: imap-send: mark unused parameters with NO_OPENSSL
  [17/22]: grep: mark unused parmaeters in pcre fallbacks
  [18/22]: credential: mark unused parameter in urlmatch callback
  [19/22]: fetch: mark unused parameter in ref_transaction callback
  [20/22]: bundle-uri: mark unused parameters in callbacks
  [21/22]: gc: mark unused descriptors in scheduler callbacks
  [22/22]: update-ref: mark unused parameter in parser callbacks

 add-interactive.c       |  8 ++++----
 builtin/fetch.c         |  2 +-
 builtin/gc.c            |  6 +++---
 builtin/ls-tree.c       |  3 ++-
 builtin/stash.c         |  2 +-
 builtin/update-ref.c    | 14 +++++++-------
 builtin/worktree.c      |  8 ++++----
 bundle-uri.c            |  6 +++---
 commit-graph.c          |  8 +++++---
 credential.c            |  4 ++--
 grep.c                  | 12 +++++++-----
 imap-send.c             | 10 ++++++++--
 negotiator/noop.c       | 12 +++++++-----
 pack-bitmap.c           |  5 +++--
 ref-filter.c            |  8 +++++---
 sequencer.c             | 27 ++++++++++++++-------------
 t/helper/test-trace2.c  |  6 +++---
 trace2/tr2_sysenv.c     |  3 ++-
 trace2/tr2_tgt_event.c  | 23 +++++++++++++----------
 trace2/tr2_tgt_normal.c | 20 ++++++++++++--------
 worktree.c              |  6 ++++--
 21 files changed, 110 insertions(+), 83 deletions(-)

-Peff

Jeff King· Aug 28, 2023, 21:46 UTC · re: Jeff King · lore

[PATCH 01/22] sequencer: use repository parameter in short_commit_name()

Instead of just using the_repository, we can take a repository parameter from the caller. Most of them already have one, and doing so clears up a few -Wunused-parameter warnings. There are still a few callers which use the_repository, but this pushes us one small step forward to eventually getting rid of those.

Signed-off-by: Jeff King <peff@peff.net>
---
 sequencer.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Show changes to sequencer.c +13 −12

diff --git a/sequencer.c b/sequencer.c
index 48475d1cc6..82dc3e160e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -433,10 +433,9 @@ struct commit_message {
 	const char *message;
 };
 
-static const char *short_commit_name(struct commit *commit)
+static const char *short_commit_name(struct repository *r, struct commit *commit)
 {
-	return repo_find_unique_abbrev(the_repository, &commit->object.oid,
-				       DEFAULT_ABBREV);
+	return repo_find_unique_abbrev(r, &commit->object.oid, DEFAULT_ABBREV);
 }
 
 static int get_message(struct commit *commit, struct commit_message *out)
@@ -446,7 +445,7 @@ static int get_message(struct commit *commit, struct commit_message *out)
 
 	out->message = repo_logmsg_reencode(the_repository, commit, NULL,
 					    get_commit_output_encoding());
-	abbrev = short_commit_name(commit);
+	abbrev = short_commit_name(the_repository, commit);
 
 	subject_len = find_commit_subject(out->message, &subject);
 
@@ -2383,7 +2382,7 @@ static int do_pick_commit(struct repository *r,
 		error(command == TODO_REVERT
 		      ? _("could not revert %s... %s")
 		      : _("could not apply %s... %s"),
-		      short_commit_name(commit), msg.subject);
+		      short_commit_name(r, commit), msg.subject);
 		print_advice(r, res == 1, opts);
 		repo_rerere(r, opts->allow_rerere_auto);
 		goto leave;
@@ -3172,7 +3171,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
 		item->offset_in_buf = todo_list->buf.len;
 		subject_len = find_commit_subject(commit_buffer, &subject);
 		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
-			short_commit_name(commit), subject_len, subject);
+			short_commit_name(opts->revs->repo, commit),
+			subject_len, subject);
 		repo_unuse_commit_buffer(the_repository, commit,
 					 commit_buffer);
 	}
@@ -3593,7 +3593,7 @@ static int error_with_patch(struct repository *r,
 	} else if (exit_code) {
 		if (commit)
 			fprintf_ln(stderr, _("Could not apply %s... %.*s"),
-				   short_commit_name(commit), subject_len, subject);
+				   short_commit_name(r, commit), subject_len, subject);
 		else
 			/*
 			 * We don't have the hash of the parent so
@@ -4728,7 +4728,7 @@ static int pick_commits(struct repository *r,
 						term_clear_line();
 					fprintf(stderr,
 						_("Stopped at %s...  %.*s\n"),
-						short_commit_name(commit),
+						short_commit_name(r, commit),
 						item->arg_len, arg);
 				}
 				return error_with_patch(r, commit,
@@ -5564,7 +5564,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 		if (!is_empty && (commit->object.flags & PATCHSAME)) {
 			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
 				warning(_("skipped previously applied commit %s"),
-					short_commit_name(commit));
+					short_commit_name(revs->repo, commit));
 			skipped_commit = 1;
 			continue;
 		}
@@ -5800,7 +5800,7 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 		if (!is_empty && (commit->object.flags & PATCHSAME)) {
 			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
 				warning(_("skipped previously applied commit %s"),
-					short_commit_name(commit));
+					short_commit_name(r, commit));
 			skipped_commit = 1;
 			continue;
 		}
@@ -5892,7 +5892,8 @@ static void todo_list_add_exec_commands(struct todo_list *todo_list,
 	todo_list->alloc = alloc;
 }
 
-static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,
+static void todo_list_to_strbuf(struct repository *r,
+				struct todo_list *todo_list,
 				struct strbuf *buf, int num, unsigned flags)
 {
 	struct todo_item *item;
@@ -5921,7 +5922,7 @@ static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_lis
 		/* add commit id */
 		if (item->commit) {
 			const char *oid = flags & TODO_LIST_SHORTEN_IDS ?
-					  short_commit_name(item->commit) :
+					  short_commit_name(r, item->commit) :
 					  oid_to_hex(&item->commit->object.oid);
 
 			if (item->command == TODO_FIXUP) {

-- 
2.42.0.505.g4c6fb48dec

Junio C Hamano· Aug 28, 2023, 22:21 UTC · re: Jeff King · lore

Re: [PATCH 01/22] sequencer: use repository parameter in short_commit_name()

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> -static const char *short_commit_name(struct commit *commit)
> +static const char *short_commit_name(struct repository *r, struct commit *commit)
>  {
> -	return repo_find_unique_abbrev(the_repository, &commit->object.oid,
> -				       DEFAULT_ABBREV);
> +	return repo_find_unique_abbrev(r, &commit->object.oid, DEFAULT_ABBREV);
>  }

Theoretically this is the right thing to do, and ...

Show 7 quoted lines

>  static int get_message(struct commit *commit, struct commit_message *out)
> @@ -446,7 +445,7 @@ static int get_message(struct commit *commit, struct commit_message *out)
>  
>  	out->message = repo_logmsg_reencode(the_repository, commit, NULL,
>  					    get_commit_output_encoding());
> -	abbrev = short_commit_name(commit);
> +	abbrev = short_commit_name(the_repository, commit);

... this is a no-op.

Show 6 quoted lines

> @@ -2383,7 +2382,7 @@ static int do_pick_commit(struct repository *r,
>  		error(command == TODO_REVERT
>  		      ? _("could not revert %s... %s")
>  		      : _("could not apply %s... %s"),
> -		      short_commit_name(commit), msg.subject);
> +		      short_commit_name(r, commit), msg.subject);

And this is a logical consequence for a function that is told by the caller in which repository to work in via its parameter.

Show 9 quoted lines

> @@ -3172,7 +3171,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
>  		item->offset_in_buf = todo_list->buf.len;
>  		subject_len = find_commit_subject(commit_buffer, &subject);
>  		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
> -			short_commit_name(commit), subject_len, subject);
> +			short_commit_name(opts->revs->repo, commit),
> +			subject_len, subject);
>  		repo_unuse_commit_buffer(the_repository, commit,
>  					 commit_buffer);

But how do we ascertain that opts->revs->repo is always safe to use (iow initialized to a sensible value)? repo_init_revisions() takes a repository as its parameter and the first thing it does is to set it to the revs->repo, so it is safe for any "struct rev_info" that came from there, but REV_INFO_INIT omits initializer for the .repo member.

The other two calls in this loop refer to the_repository so the current code would be safe even if opts->revs->repo is set or NULL, but that will no longer be true with the updated code. I'd feel safer if at the beginning of the function we create a local variable "struct rev_info *repo" that is initialized to opts->revs->repo and use it throughout the function instead of the_repository.

Show 9 quoted lines

> @@ -5564,7 +5564,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
>  		if (!is_empty && (commit->object.flags & PATCHSAME)) {
>  			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
>  				warning(_("skipped previously applied commit %s"),
> -					short_commit_name(commit));
> +					short_commit_name(revs->repo, commit));
>  			skipped_commit = 1;
>  			continue;
>  		}

This one I am fairly certain is a safe and correct conversion; the function would be segfaulting in its call to get_revision() if revs->repo were set to a bogus value.

Thanks.

Taylor Blau· Aug 28, 2023, 23:06 UTC · re: Junio C Hamano · lore

Re: [PATCH 01/22] sequencer: use repository parameter in short_commit_name()

On Mon, Aug 28, 2023 at 03:21:11PM -0700, Junio C Hamano wrote:

Show 23 quoted lines

> > @@ -3172,7 +3171,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
> >  		item->offset_in_buf = todo_list->buf.len;
> >  		subject_len = find_commit_subject(commit_buffer, &subject);
> >  		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
> > -			short_commit_name(commit), subject_len, subject);
> > +			short_commit_name(opts->revs->repo, commit),
> > +			subject_len, subject);
> >  		repo_unuse_commit_buffer(the_repository, commit,
> >  					 commit_buffer);
>
> But how do we ascertain that opts->revs->repo is always safe to use
> (iow initialized to a sensible value)?  repo_init_revisions() takes
> a repository as its parameter and the first thing it does is to set
> it to the revs->repo, so it is safe for any "struct rev_info" that
> came from there, but REV_INFO_INIT omits initializer for the .repo
> member.
>
> The other two calls in this loop refer to the_repository so the
> current code would be safe even if opts->revs->repo is set or NULL,
> but that will no longer be true with the updated code.  I'd feel
> safer if at the beginning of the function we create a local variable
> "struct rev_info *repo" that is initialized to opts->revs->repo and
> use it throughout the function instead of the_repository.

This call comes from sequencer_pick_revisions() -> walk_revs_populate_todo(), where opts is passed in from the caller of the former function.

The sole caller of that function is run_sequencer() in builtin/revert.c. The relevant portion of that function is:

    if (cmd) {
        opts->revs = NULL;
    } else {
        struct setup_revision_opt s_r_opt;
        opts->revs = xmalloc(sizeof(*opts->revs));
        repo_init_revisions(the_repository, opts->revs, NULL);
        opts->revs->no_walk = 1;
        opts->revs->unsorted_input = 1;
        /* ... */
    }

So as long as we end up in the else arm of this conditional, we'll have called repo_init_revisions(), causing opts->revs->repo to be equal to the_repository.

Thankfully, we have an assert(opts->revs) at the beginning of sequencer_pick_revisions(), so we know that we always take the else arm on this particular call path.

...but, sequencer_pick_revisions() itself takes a repository pointer, so we could equally use that, or drop it, like so:

--- 8< ---

Show changes to 3 files +10 −9

builtin/revert.c, sequencer.c, sequencer.h

diff --git a/builtin/revert.c b/builtin/revert.c
index e6f9a1ad26..29e215c72a 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -224,7 +224,7 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		return sequencer_rollback(the_repository, opts);
 	if (cmd == 's')
 		return sequencer_skip(the_repository, opts);
-	return sequencer_pick_revisions(the_repository, opts);
+	return sequencer_pick_revisions(opts);
 }

 int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/sequencer.c b/sequencer.c
index 48475d1cc6..bc7e687623 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -5201,14 +5201,15 @@ static int single_pick(struct repository *r,
 	return do_pick_commit(r, &item, opts, 0, &check_todo);
 }

-int sequencer_pick_revisions(struct repository *r,
-			     struct replay_opts *opts)
+int sequencer_pick_revisions(struct replay_opts *opts)
 {
 	struct todo_list todo_list = TODO_LIST_INIT;
 	struct object_id oid;
+	struct repository *r;
 	int i, res;

 	assert(opts->revs);
+	r = opts->revs->repo;
 	if (read_and_refresh_cache(r, opts))
 		return -1;

diff --git a/sequencer.h b/sequencer.h
index 913a0f652d..10caa3dc93 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -158,8 +158,7 @@ void todo_list_filter_update_refs(struct repository *r,

 /* Call this to setup defaults before parsing command line options */
 void sequencer_init_config(struct replay_opts *opts);
-int sequencer_pick_revisions(struct repository *repo,
-			     struct replay_opts *opts);
+int sequencer_pick_revisions(struct replay_opts *opts);
 int sequencer_continue(struct repository *repo, struct replay_opts *opts);
 int sequencer_rollback(struct repository *repo, struct replay_opts *opts);
 int sequencer_skip(struct repository *repo, struct replay_opts *opts);
--- >8 ---

though it makes for an awkward API to have all of the other
sequencer-related functions take as their first argument a pointer to a
repository struct, leaving sequencer_pick_revisions() as the odd one
out.

So I'd probably just as soon drop that and do what Junio suggests:

--- 8< ---
diff --git a/sequencer.c b/sequencer.c
index 82dc3e160e..6c06b8e1bb 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3143,22 +3143,24 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
 }

 static int walk_revs_populate_todo(struct todo_list *todo_list,
-				struct replay_opts *opts)
+				   struct replay_opts *opts)
 {
 	enum todo_command command = opts->action == REPLAY_PICK ?
 		TODO_PICK : TODO_REVERT;
 	const char *command_string = todo_command_info[command].str;
 	const char *encoding;
 	struct commit *commit;
+	struct repository *r;

 	if (prepare_revs(opts))
 		return -1;
+	r = opts->revs->repo;

 	encoding = get_log_output_encoding();

 	while ((commit = get_revision(opts->revs))) {
 		struct todo_item *item = append_new_todo(todo_list);
-		const char *commit_buffer = repo_logmsg_reencode(the_repository,
+		const char *commit_buffer = repo_logmsg_reencode(r,
 								 commit, NULL,
 								 encoding);
 		const char *subject;
@@ -3173,8 +3175,7 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
 		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
 			short_commit_name(opts->revs->repo, commit),
 			subject_len, subject);
-		repo_unuse_commit_buffer(the_repository, commit,
-					 commit_buffer);
+		repo_unuse_commit_buffer(r, commit, commit_buffer);
 	}

 	if (!todo_list->nr)
--- >8 ---

> > @@ -5564,7 +5564,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
> >  		if (!is_empty && (commit->object.flags & PATCHSAME)) {
> >  			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
> >  				warning(_("skipped previously applied commit %s"),
> > -					short_commit_name(commit));
> > +					short_commit_name(revs->repo, commit));
> >  			skipped_commit = 1;
> >  			continue;
> >  		}
>
> This one I am fairly certain is a safe and correct conversion; the
> function would be segfaulting in its call to get_revision() if
> revs->repo were set to a bogus value.

Agreed.

Thanks,
Taylor

Jeff King· Aug 29, 2023, 00:48 UTC · re: Junio C Hamano · lore

Re: [PATCH 01/22] sequencer: use repository parameter in short_commit_name()

On Mon, Aug 28, 2023 at 03:21:11PM -0700, Junio C Hamano wrote:

Show 16 quoted lines

> > @@ -3172,7 +3171,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
> >  		item->offset_in_buf = todo_list->buf.len;
> >  		subject_len = find_commit_subject(commit_buffer, &subject);
> >  		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
> > -			short_commit_name(commit), subject_len, subject);
> > +			short_commit_name(opts->revs->repo, commit),
> > +			subject_len, subject);
> >  		repo_unuse_commit_buffer(the_repository, commit,
> >  					 commit_buffer);
> 
> But how do we ascertain that opts->revs->repo is always safe to use
> (iow initialized to a sensible value)?  repo_init_revisions() takes
> a repository as its parameter and the first thing it does is to set
> it to the revs->repo, so it is safe for any "struct rev_info" that
> came from there, but REV_INFO_INIT omits initializer for the .repo
> member.

I mostly just assumed it would have been initialized, because passing in anything else and expecting it to work is a bit crazy. REV_INFO_INIT specifically says:

   * This will not fully initialize a "struct rev_info", the
   * repo_init_revisions() function needs to be called before
   * setup_revisions() and any revision walking takes place.

and then goes on to explain that the point is just so you can "goto cleanup" and free it if you never made it to the init_revisions() call. So the code would already be pretty buggy in this case.

That said, all of the code paths here do call repo_init_revisions(), so I think it is OK. But if we want to make the patch simple and more obviously correct, I would prefer to just blindly use the_repository in callers that don't have a "struct repository" themselves. Then we know it's a strict step forward (if slightly smaller), and we can leave the refactoring of the rest of the sequencer code for another day. I was trying hard to draw a reasonable line and not get this already-large series derailed by only semi-related refactoring.

Show 6 quoted lines

> The other two calls in this loop refer to the_repository so the
> current code would be safe even if opts->revs->repo is set or NULL,
> but that will no longer be true with the updated code.  I'd feel
> safer if at the beginning of the function we create a local variable
> "struct rev_info *repo" that is initialized to opts->revs->repo and
> use it throughout the function instead of the_repository.

I'm not sure how that makes it any safer, as it would mean using the suspect repo pointer in more places. Unless you are proposing to do:

  struct repository *r = opts->revs->repo;
  if (!r)
	r = the_repository; /* or BUG() ? */

-Peff

Junio C Hamano· Aug 29, 2023, 01:16 UTC · re: Jeff King · lore

Re: [PATCH 01/22] sequencer: use repository parameter in short_commit_name()

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

>> but that will no longer be true with the updated code.  I'd feel
>> safer if at the beginning of the function we create a local variable
>> "struct rev_info *repo" that is initialized to opts->revs->repo and
>> use it throughout the function instead of the_repository.
>
> I'm not sure how that makes it any safer, as it would mean using the
> suspect repo pointer in more places. Unless you are proposing to do:
>
>   struct repository *r = opts->revs->repo;
>   if (!r)
> 	r = the_repository; /* or BUG() ? */

Yup, the BUG() variant was exactly what I had in mind, but without the assert, by using opts->revs->repo more consistently, we would make sure nobody will call us with uninitialized opts->revs->repo now or in the future.

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 02/22] sequencer: mark repository argument as unused

In sequencer_get_last_command(), we don't ever look at the repository parameter. It _should_ be used when calling into git_path_* functions, but the one we use here is declared with the non-REPO variant of GIT_PATH_FUNC(), and so just uses the_repository internally.

We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in sequencer.c, and inconsistently switching one makes the code more confusing. Likewise, this one function is used in half a dozen other spots, all of which would need to start passing in a repository argument (with rippling effects up the call stack).

So let's punt on that for now and just silence any -Wunused-parameter warning.

Note that we could also drop this parameter entirely, as the function is always called directly, and not as a callback that has to conform to some external interface. But since we'd eventually want to use the repository parameter, let's leave it in place to avoid disrupting the callers twice.

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

Show changes to sequencer.c +1 −1

diff --git a/sequencer.c b/sequencer.c
index 82dc3e160e..41fd79d215 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2649,7 +2649,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
 	return item->commit ? 0 : -1;
 }
 
-int sequencer_get_last_command(struct repository *r, enum replay_action *action)
+int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action)
 {
 	const char *todo_file, *bol;
 	struct strbuf buf = STRBUF_INIT;

-- 
2.42.0.505.g4c6fb48dec

Taylor Blau· Aug 28, 2023, 23:24 UTC · re: Jeff King · lore

Re: [PATCH 02/22] sequencer: mark repository argument as unused

On Mon, Aug 28, 2023 at 05:47:17PM -0400, Jeff King wrote:

Show 5 quoted lines

> Note that we could also drop this parameter entirely, as the function is
> always called directly, and not as a callback that has to conform to
> some external interface. But since we'd eventually want to use the
> repository parameter, let's leave it in place to avoid disrupting the
> callers twice.

Yeah, I think that this (and the discussion on the previous patch) are hinting that we should consider dropping the repository argument more broadly throughout the sequencer API, since we can often use `opts->revs->repo` in its place.

But we can definitely leave that for another day ;-). In the meantime, this patch LGTM.

Thanks, Taylor

Phillip Wood· Aug 29, 2023, 15:55 UTC · re: Jeff King · lore

Re: [PATCH 02/22] sequencer: mark repository argument as unused

Hi Peff

On 28/08/2023 22:47, Jeff King wrote:

Show 8 quoted lines

> In sequencer_get_last_command(), we don't ever look at the repository
> parameter. It _should_ be used when calling into git_path_* functions,
> but the one we use here is declared with the non-REPO variant of
> GIT_PATH_FUNC(), and so just uses the_repository internally.
> 
> We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing
> so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in
> sequencer.c,

Wow, I knew there were quite a few but I hadn't realized there were that many. Changing them all to take a struct repository will be a big change and will make struct repo_cache_path a lot larger.

Show 13 quoted lines

> and inconsistently switching one makes the code more
> confusing. Likewise, this one function is used in half a dozen other
> spots, all of which would need to start passing in a repository argument
> (with rippling effects up the call stack).
> 
> So let's punt on that for now and just silence any -Wunused-parameter
> warning.
> 
> Note that we could also drop this parameter entirely, as the function is
> always called directly, and not as a callback that has to conform to
> some external interface. But since we'd eventually want to use the
> repository parameter, let's leave it in place to avoid disrupting the
> callers twice.

I think that makes sense as we're going to need that argument eventually. I was curious as to why this function takes a repository argument. When the function was added in 4a72486de97 (fix cherry-pick/revert status after commit, 2019-04-16) it called parse_insn_line() which takes a repository argument. It was refactored in ed5b1ca10b (status: do not report errors in sequencer/todo, 2019-06-27) and I failed to notice that the repository was unused afterwards.

Best Wishes

Phillip

Show 18 quoted lines

> Signed-off-by: Jeff King <peff@peff.net>
> ---
>   sequencer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 82dc3e160e..41fd79d215 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2649,7 +2649,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
>   	return item->commit ? 0 : -1;
>   }
>   
> -int sequencer_get_last_command(struct repository *r, enum replay_action *action)
> +int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action)
>   {
>   	const char *todo_file, *bol;
>   	struct strbuf buf = STRBUF_INIT;

Jeff King· Aug 29, 2023, 23:32 UTC · re: Phillip Wood · lore

Re: [PATCH 02/22] sequencer: mark repository argument as unused

On Tue, Aug 29, 2023 at 04:55:30PM +0100, Phillip Wood wrote:

Show 7 quoted lines

> > We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing
> > so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in
> > sequencer.c,
> 
> Wow, I knew there were quite a few but I hadn't realized there were that
> many. Changing them all to take a struct repository will be a big change and
> will make struct repo_cache_path a lot larger.

Yeah. One of the things that gave me pause is that some of them may need to be renamed, as well. Most of the existing ones are static local within sequencer.c, so names like git_path_head_file() are OK. But REPO_GIT_PATH_FUNC() requires a pointer in the global repo_cache_path, so the names need to be appropriate for a global view.

Show 13 quoted lines

> > Note that we could also drop this parameter entirely, as the function is
> > always called directly, and not as a callback that has to conform to
> > some external interface. But since we'd eventually want to use the
> > repository parameter, let's leave it in place to avoid disrupting the
> > callers twice.
> 
> I think that makes sense as we're going to need that argument eventually. I
> was curious as to why this function takes a repository argument. When the
> function was added in 4a72486de97 (fix cherry-pick/revert status after
> commit, 2019-04-16) it called parse_insn_line() which takes a repository
> argument. It was refactored in ed5b1ca10b (status: do not report errors in
> sequencer/todo, 2019-06-27) and I failed to notice that the repository was
> unused afterwards.

Thanks for digging there. I usually do something similar for these patches (to help verify that there's no extra bug lurking), but with "struct repository" ones, the answer was usually uninteresting ("it should be used, but the underlying functions don't support it" kind of thing).

Since I'm re-rolling anyway, I'll throw your research into the commit message.

-Peff

Phillip Wood· Aug 30, 2023, 13:35 UTC · re: Jeff King · lore

Re: [PATCH 02/22] sequencer: mark repository argument as unused

On 30/08/2023 00:32, Jeff King wrote:

Show 15 quoted lines

> On Tue, Aug 29, 2023 at 04:55:30PM +0100, Phillip Wood wrote:
> 
>>> We could change the path helper to use REPO_GIT_PATH_FUNC(), but doing
>>> so piecemeal is not great. There are 41 uses of GIT_PATH_FUNC() in
>>> sequencer.c,
>>
>> Wow, I knew there were quite a few but I hadn't realized there were that
>> many. Changing them all to take a struct repository will be a big change and
>> will make struct repo_cache_path a lot larger.
> 
> Yeah. One of the things that gave me pause is that some of them may need
> to be renamed, as well. Most of the existing ones are static local
> within sequencer.c, so names like git_path_head_file() are OK. But
> REPO_GIT_PATH_FUNC() requires a pointer in the global repo_cache_path,
> so the names need to be appropriate for a global view.

It's definitely worth leaving that for a separate patch series. A lot of the paths start rebase_path_... (I think it is only the paths used by cherry-pick/revert that start git_path_...) so we'd need a separate macro unless we renamed them. Having a different prefix is probably a good thing for making them global functions but it still leaves the problem of renaming the ones like git_path_head_file().

Best Wishes

Phillip

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 04/22] pack-bitmap: mark unused parameters in show_object callback

This is similar to the cases in c50dca2a18 (list-objects: mark unused callback parameters, 2023-02-24), but was added after that commit.

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

Show changes to pack-bitmap.c +3 −2

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 6afc03d1e4..ca8319b87c 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1101,8 +1101,9 @@ static void show_boundary_commit(struct commit *commit, void *_data)
 	}
 }
 
-static void show_boundary_object(struct object *object,
-				 const char *name, void *data)
+static void show_boundary_object(struct object *object UNUSED,
+				 const char *name UNUSED,
+				 void *data UNUSED)
 {
 	BUG("should not be called");
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 03/22] ref-filter: mark unused parameters in parser callbacks

These are similar to the cases annotated in 5fe9e1ce2f (ref-filter: mark unused callback parameters, 2023-02-24), but were added after that commit.

Note that the ahead/behind callback ignores its "atom" parameter, which is a little unusual, since that struct usually stores the result. But in this case, the data is stored centrally in ref_array->counts, since we want to compute all ahead/behinds at once, not per ref.

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

Show changes to ref-filter.c +5 −3

diff --git a/ref-filter.c b/ref-filter.c
index 1bfaf20fbf..88b021dd1d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -549,7 +549,8 @@ static int signature_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
+static int trailers_atom_parser(struct ref_format *format UNUSED,
+				struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
 	atom->u.contents.trailer_opts.no_divider = 1;
@@ -819,7 +820,7 @@ static int if_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int rest_atom_parser(struct ref_format *format,
+static int rest_atom_parser(struct ref_format *format UNUSED,
 			    struct used_atom *atom UNUSED,
 			    const char *arg, struct strbuf *err)
 {
@@ -828,7 +829,8 @@ static int rest_atom_parser(struct ref_format *format,
 	return 0;
 }
 
-static int ahead_behind_atom_parser(struct ref_format *format, struct used_atom *atom,
+static int ahead_behind_atom_parser(struct ref_format *format,
+				    struct used_atom *atom UNUSED,
 				    const char *arg, struct strbuf *err)
 {
 	struct string_list_item *item;

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 08/22] stash: mark unused parameter in diff callback

This is similar to the cases in 61bdc7c5d8 (diff: mark unused parameters in callbacks, 2022-12-13), but I missed it when making that commit.

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

Show changes to builtin/stash.c +1 −1

diff --git a/builtin/stash.c b/builtin/stash.c
index fe64cde9ce..c9365bea13 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -362,7 +362,7 @@ static int is_path_a_directory(const char *path)
 }
 
 static void add_diff_to_buf(struct diff_queue_struct *q,
-			    struct diff_options *options,
+			    struct diff_options *options UNUSED,
 			    void *data)
 {
 	int i;

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 05/22] worktree: mark unused parameters in each_ref_fn callback

This is similar to the cases in 63e14ee2d6 (refs: mark unused each_ref_fn parameters, 2022-08-19), but it was added after that commit.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/worktree.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to builtin/worktree.c +4 −4

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 10db70b7ec..62b7e26f4b 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -628,10 +628,10 @@ static void print_preparing_worktree_line(int detach,
  *
  * Returns 0 on failure and non-zero on success.
  */
-static int first_valid_ref(const char *refname,
-			   const struct object_id *oid,
-			   int flags,
-			   void *cb_data)
+static int first_valid_ref(const char *refname UNUSED,
+			   const struct object_id *oid UNUSED,
+			   int flags UNUSED,
+			   void *cb_data UNUSED)
 {
 	return 1;
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 06/22] commit-graph: mark unused data parameters in generation callbacks

The compute_generation_info code uses function pointers to abstract the get/set generation operations. Some callers don't need the extra void data pointer, which should be annotated to appease -Wunused-parameter.

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

Show changes to commit-graph.c +5 −3

diff --git a/commit-graph.c b/commit-graph.c
index 0aa1640d15..73a539495b 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1568,12 +1568,14 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx)
 	stop_progress(&ctx->progress);
 }
 
-static timestamp_t get_generation_from_graph_data(struct commit *c, void *data)
+static timestamp_t get_generation_from_graph_data(struct commit *c,
+						  void *data UNUSED)
 {
 	return commit_graph_data_at(c)->generation;
 }
 
-static void set_generation_v2(struct commit *c, timestamp_t t, void *data)
+static void set_generation_v2(struct commit *c, timestamp_t t,
+			      void *data UNUSED)
 {
 	struct commit_graph_data *g = commit_graph_data_at(c);
 	g->generation = t;
@@ -1616,7 +1618,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 }
 
 static void set_generation_in_graph_data(struct commit *c, timestamp_t t,
-					 void *data)
+					 void *data UNUSED)
 {
 	commit_graph_data_at(c)->generation = t;
 }

-- 
2.42.0.505.g4c6fb48dec

Taylor Blau· Aug 28, 2023, 23:32 UTC · re: Jeff King · lore

Re: [PATCH 06/22] commit-graph: mark unused data parameters in generation callbacks

On Mon, Aug 28, 2023 at 05:47:39PM -0400, Jeff King wrote:

> The compute_generation_info code uses function pointers to abstract the
> get/set generation operations. Some callers don't need the extra void
> data pointer, which should be annotated to appease -Wunused-parameter.

I think just the callbacks initialized by compute_topological_levels() (i.e, get_topo_level() and set_topo_level()) care about the ctx parameter.

I think that we can go a bit further, though. The other caller in compute_generation_numbers() assigns data to the argument ctx it takes in, but neither of its callbacks get_generation_from_graph_data() and set_generation_v2() use the data parameter, as is implied by the the existence of this patch.

So I think that we could drop the assignment to data entirely like so, applied on top of this patch:

--- 8< ---

Show changes to commit-graph.c +0 −1

diff --git a/commit-graph.c b/commit-graph.c
index 0aa1640d15..eb3e27b720 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1587,7 +1587,6 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 		.commits = &ctx->commits,
 		.get_generation = get_generation_from_graph_data,
 		.set_generation = set_generation_v2,
-		.data = ctx,
 	};

 	if (ctx->report_progress)
--- >8 ---

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

The rest looks obviously correct to me, thanks.

Thanks,
Taylor

Jeff King· Aug 29, 2023, 00:52 UTC · re: Taylor Blau · lore

Re: [PATCH 06/22] commit-graph: mark unused data parameters in generation callbacks

On Mon, Aug 28, 2023 at 07:32:59PM -0400, Taylor Blau wrote:

Show 32 quoted lines

> On Mon, Aug 28, 2023 at 05:47:39PM -0400, Jeff King wrote:
> > The compute_generation_info code uses function pointers to abstract the
> > get/set generation operations. Some callers don't need the extra void
> > data pointer, which should be annotated to appease -Wunused-parameter.
> 
> I think just the callbacks initialized by compute_topological_levels()
> (i.e, get_topo_level() and set_topo_level()) care about the ctx
> parameter.
> 
> I think that we can go a bit further, though. The other caller in
> compute_generation_numbers() assigns data to the argument ctx it takes
> in, but neither of its callbacks get_generation_from_graph_data() and
> set_generation_v2() use the data parameter, as is implied by the the
> existence of this patch.
> 
> So I think that we could drop the assignment to data entirely like so,
> applied on top of this patch:
> 
> --- 8< ---
> diff --git a/commit-graph.c b/commit-graph.c
> index 0aa1640d15..eb3e27b720 100644
> --- a/commit-graph.c
> +++ b/commit-graph.c
> @@ -1587,7 +1587,6 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
>  		.commits = &ctx->commits,
>  		.get_generation = get_generation_from_graph_data,
>  		.set_generation = set_generation_v2,
> -		.data = ctx,
>  	};
> 
>  	if (ctx->report_progress)
> --- >8 ---

Yeah, I didn't dig too much in the caller. I could see an argument for leaving it, in that it might be the "least surprise" for somebody who later wants to look at the ctx variable from those callbacks. But since all they get is a void pointer, anybody writing that code would have to go back to the caller to see what is passed in as "data" anyway.

So I think it is safe to add this cleanup. I'm going to re-roll to simplify the first patch a bit, so I'll add this in as well. Thanks.

-Peff

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 07/22] ls-tree: mark unused parameter in callback

The formatting functions are dispatched from a table of function pointers. The "path name only" function unsurprisingly does not need to look at its "oid" parameter, but we must mark it as unused to make -Wunused-parameter happy.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/ls-tree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Show changes to builtin/ls-tree.c +2 −1

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index f558db5f3b..209d2dc0d5 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -241,7 +241,8 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 	return recurse;
 }
 
-static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
+static int show_tree_name_only(const struct object_id *oid UNUSED,
+			       struct strbuf *base,
 			       const char *pathname, unsigned mode,
 			       void *context)
 {

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 09/22] trace2: mark unused us_elapsed_absolute parameters

Many trace2 targets ignore the absolute elapsed time parameters. However, the virtual interface needs to retain the parameter since it is used by others (e.g., the perf target).

Signed-off-by: Jeff King <peff@peff.net>
---
 trace2/tr2_tgt_event.c  | 23 +++++++++++++----------
 trace2/tr2_tgt_normal.c | 20 ++++++++++++--------
 2 files changed, 25 insertions(+), 18 deletions(-)

Show changes to 2 files +25 −18

trace2/tr2_tgt_event.c, trace2/tr2_tgt_normal.c

diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index 53091781ec..59910a1a4f 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c
@@ -335,7 +335,7 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
 }
 
 static void fn_child_start_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      const struct child_process *cmd)
 {
 	const char *event_name = "child_start";
@@ -367,7 +367,8 @@ static void fn_child_start_fl(const char *file, int line,
 }
 
 static void fn_child_exit_fl(const char *file, int line,
-			     uint64_t us_elapsed_absolute, int cid, int pid,
+			     uint64_t us_elapsed_absolute UNUSED,
+			     int cid, int pid,
 			     int code, uint64_t us_elapsed_child)
 {
 	const char *event_name = "child_exit";
@@ -388,7 +389,8 @@ static void fn_child_exit_fl(const char *file, int line,
 }
 
 static void fn_child_ready_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int cid, int pid,
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int cid, int pid,
 			      const char *ready, uint64_t us_elapsed_child)
 {
 	const char *event_name = "child_ready";
@@ -409,7 +411,7 @@ static void fn_child_ready_fl(const char *file, int line,
 }
 
 static void fn_thread_start_fl(const char *file, int line,
-			       uint64_t us_elapsed_absolute)
+			       uint64_t us_elapsed_absolute UNUSED)
 {
 	const char *event_name = "thread_start";
 	struct json_writer jw = JSON_WRITER_INIT;
@@ -423,7 +425,7 @@ static void fn_thread_start_fl(const char *file, int line,
 }
 
 static void fn_thread_exit_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      uint64_t us_elapsed_thread)
 {
 	const char *event_name = "thread_exit";
@@ -439,7 +441,8 @@ static void fn_thread_exit_fl(const char *file, int line,
 	jw_release(&jw);
 }
 
-static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
+static void fn_exec_fl(const char *file, int line,
+		       uint64_t us_elapsed_absolute UNUSED,
 		       int exec_id, const char *exe, const char **argv)
 {
 	const char *event_name = "exec";
@@ -460,8 +463,8 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
 }
 
 static void fn_exec_result_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int exec_id,
-			      int code)
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int exec_id, int code)
 {
 	const char *event_name = "exec_result";
 	struct json_writer jw = JSON_WRITER_INIT;
@@ -511,7 +514,7 @@ static void fn_repo_fl(const char *file, int line,
 }
 
 static void fn_region_enter_printf_va_fl(const char *file, int line,
-					 uint64_t us_elapsed_absolute,
+					 uint64_t us_elapsed_absolute UNUSED,
 					 const char *category,
 					 const char *label,
 					 const struct repository *repo,
@@ -538,7 +541,7 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,
 }
 
 static void fn_region_leave_printf_va_fl(
-	const char *file, int line, uint64_t us_elapsed_absolute,
+	const char *file, int line, uint64_t us_elapsed_absolute UNUSED,
 	uint64_t us_elapsed_region, const char *category, const char *label,
 	const struct repository *repo, const char *fmt, va_list ap)
 {
diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index d25ea13164..38d5ebddf6 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -86,7 +86,7 @@ static void fn_version_fl(const char *file, int line)
 }
 
 static void fn_start_fl(const char *file, int line,
-			uint64_t us_elapsed_absolute, const char **argv)
+			uint64_t us_elapsed_absolute UNUSED, const char **argv)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
 
@@ -215,7 +215,7 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
 }
 
 static void fn_child_start_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      const struct child_process *cmd)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -243,7 +243,8 @@ static void fn_child_start_fl(const char *file, int line,
 }
 
 static void fn_child_exit_fl(const char *file, int line,
-			     uint64_t us_elapsed_absolute, int cid, int pid,
+			     uint64_t us_elapsed_absolute UNUSED,
+			     int cid, int pid,
 			     int code, uint64_t us_elapsed_child)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -256,7 +257,8 @@ static void fn_child_exit_fl(const char *file, int line,
 }
 
 static void fn_child_ready_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int cid, int pid,
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int cid, int pid,
 			      const char *ready, uint64_t us_elapsed_child)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -268,7 +270,8 @@ static void fn_child_ready_fl(const char *file, int line,
 	strbuf_release(&buf_payload);
 }
 
-static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
+static void fn_exec_fl(const char *file, int line,
+		       uint64_t us_elapsed_absolute UNUSED,
 		       int exec_id, const char *exe, const char **argv)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -284,8 +287,8 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
 }
 
 static void fn_exec_result_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int exec_id,
-			      int code)
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int exec_id, int code)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
 
@@ -321,7 +324,8 @@ static void fn_repo_fl(const char *file, int line,
 }
 
 static void fn_printf_va_fl(const char *file, int line,
-			    uint64_t us_elapsed_absolute, const char *fmt,
+			    uint64_t us_elapsed_absolute UNUSED,
+			    const char *fmt,
 			    va_list ap)
 {
 	struct strbuf buf_payload = STRBUF_INIT;

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 11/22] test-trace2: mark unused argv/argc parameters

The trace2 test helper uses function pointers to dispatch to individual tests. Not all tests bother looking at their argv/argc parameters. We could tighten this up (e.g., complaining when seeing unexpected parameters), but for internal test code it's not worth worrying about.

This is similar in spirit to 126e3b3d2a (t/helper: mark unused argv/argc arguments, 2023-03-28).

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

Show changes to t/helper/test-trace2.c +3 −3

diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
index 20c7495f38..d5ca0046c8 100644
--- a/t/helper/test-trace2.c
+++ b/t/helper/test-trace2.c
@@ -45,7 +45,7 @@ static int get_i(int *p_value, const char *data)
  * [] "def_param" events for all of the "interesting" pre-defined
  * config settings.
  */
-static int ut_001return(int argc, const char **argv)
+static int ut_001return(int argc UNUSED, const char **argv)
 {
 	int rc;
 
@@ -65,7 +65,7 @@ static int ut_001return(int argc, const char **argv)
  * [] "def_param" events for all of the "interesting" pre-defined
  * config settings.
  */
-static int ut_002exit(int argc, const char **argv)
+static int ut_002exit(int argc UNUSED, const char **argv)
 {
 	int rc;
 
@@ -201,7 +201,7 @@ static int ut_006data(int argc, const char **argv)
 	return 0;
 }
 
-static int ut_007BUG(int argc, const char **argv)
+static int ut_007BUG(int argc UNUSED, const char **argv UNUSED)
 {
 	/*
 	 * Exercise BUG() to ensure that the message is printed to trace2.

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 12/22] grep: mark unused parameter in output function

This is a callback used with grep_options.output, but we don't look at the grep_opt parameter, as we're just writing the output to stdout.

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

Show changes to grep.c +1 −1

diff --git a/grep.c b/grep.c
index 0904d55b24..0124eb1960 100644
--- a/grep.c
+++ b/grep.c
@@ -17,7 +17,7 @@ static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs,
 				 struct index_state *istate);
 
-static void std_output(struct grep_opt *opt, const void *buf, size_t size)
+static void std_output(struct grep_opt *opt UNUSED, const void *buf, size_t size)
 {
 	fwrite(buf, size, 1, stdout);
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 14/22] negotiator/noop: mark unused callback parameters

The noop negotiator unsurprisingly does not bother looking at any of its parameters. Mark them unused to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 negotiator/noop.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Show changes to negotiator/noop.c +7 −5

diff --git a/negotiator/noop.c b/negotiator/noop.c
index 7b72937686..de39028ab7 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -3,22 +3,24 @@
 #include "../commit.h"
 #include "../fetch-negotiator.h"
 
-static void known_common(struct fetch_negotiator *n, struct commit *c)
+static void known_common(struct fetch_negotiator *n UNUSED,
+			 struct commit *c UNUSED)
 {
 	/* do nothing */
 }
 
-static void add_tip(struct fetch_negotiator *n, struct commit *c)
+static void add_tip(struct fetch_negotiator *n UNUSED,
+		    struct commit *c UNUSED)
 {
 	/* do nothing */
 }
 
-static const struct object_id *next(struct fetch_negotiator *n)
+static const struct object_id *next(struct fetch_negotiator *n UNUSED)
 {
 	return NULL;
 }
 
-static int ack(struct fetch_negotiator *n, struct commit *c)
+static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
 {
 	/*
 	 * This negotiator does not emit any commits, so there is no commit to
@@ -28,7 +30,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return 0;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release(struct fetch_negotiator *n UNUSED)
 {
 	/* nothing to release */
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:47 UTC · re: Jeff King · lore

[PATCH 10/22] trace2: mark unused config callback parameter

This should have been part of 783a86c142 (config: mark unused callback parameters, 2022-08-19), but was missed in that commit.

Signed-off-by: Jeff King <peff@peff.net>
---
 trace2/tr2_sysenv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Show changes to trace2/tr2_sysenv.c +2 −1

diff --git a/trace2/tr2_sysenv.c b/trace2/tr2_sysenv.c
index f26ec95ab4..d3ecac2772 100644
--- a/trace2/tr2_sysenv.c
+++ b/trace2/tr2_sysenv.c
@@ -58,7 +58,8 @@ static struct tr2_sysenv_entry tr2_sysenv_settings[] = {
 /* clang-format on */
 
 static int tr2_sysenv_cb(const char *key, const char *value,
-			 const struct config_context *ctx UNUSED, void *d)
+			 const struct config_context *ctx UNUSED,
+			 void *d UNUSED)
 {
 	int k;

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 13/22] add-interactive: mark unused callback parameters

The interactive commands are dispatched from a table of abstract pointers, but not every command uses every parameter it receives. Mark the unused ones to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 add-interactive.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to add-interactive.c +4 −4

diff --git a/add-interactive.c b/add-interactive.c
index add9a1ad43..852e8e6b2f 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1021,9 +1021,9 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
-static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
-		    struct prefix_item_list *unused_files,
-		    struct list_and_choose_options *unused_opts)
+static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED,
+		    struct prefix_item_list *files UNUSED,
+		    struct list_and_choose_options *opts UNUSED)
 {
 	color_fprintf_ln(stdout, s->help_color, "status        - %s",
 			 _("show paths with changes"));
@@ -1074,7 +1074,7 @@ struct print_command_item_data {
 	const char *color, *reset;
 };
 
-static void print_command_item(int i, int selected,
+static void print_command_item(int i, int selected UNUSED,
 			       struct string_list_item *item,
 			       void *print_command_item_data)
 {

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 15/22] worktree: mark unused parameters in noop repair callback

The noop repair callback unsurprisingly does not look at any of its parameters. Mark them as unused to silence -Wunused-parameter.

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

Show changes to worktree.c +4 −2

diff --git a/worktree.c b/worktree.c
index b8cf29e6a1..a56a6c2a3d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -581,8 +581,10 @@ static void repair_gitfile(struct worktree *wt,
 	strbuf_release(&dotgit);
 }
 
-static void repair_noop(int iserr, const char *path, const char *msg,
-			void *cb_data)
+static void repair_noop(int iserr UNUSED,
+			const char *path UNUSED,
+			const char *msg UNUSED,
+			void *cb_data UNUSED)
 {
 	/* nothing */
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 16/22] imap-send: mark unused parameters with NO_OPENSSL

Earlier patches annotating unused parameters in imap-send missed a few cases in code that is compiled only with NO_OPENSSL. These need to retain the extra parameters to match the interfaces used when we compile with openssl support.

Note in the case of socket_perror() that the function declaration and parts of its code are shared between the two cases, and only the openssl code looks at "sock". So we can't simply mark the parameter as always unused. Instead, we can add a noop statement that references it. This is ugly, but should be portable.

Signed-off-by: Jeff King <peff@peff.net>
---
 imap-send.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Show changes to imap-send.c +8 −2

diff --git a/imap-send.c b/imap-send.c
index 06386e0b3b..996651e4f8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -206,10 +206,14 @@ static void socket_perror(const char *func, struct imap_socket *sock, int ret)
 		else
 			fprintf(stderr, "%s: unexpected EOF\n", func);
 	}
+	/* mark as used to appease -Wunused-parameter with NO_OPENSSL */
+	(void)sock;
 }
 
 #ifdef NO_OPENSSL
-static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int verify)
+static int ssl_socket_connect(struct imap_socket *sock UNUSED,
+			      int use_tls_only UNUSED,
+			      int verify UNUSED)
 {
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
@@ -904,7 +908,9 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
 
 #else
 
-static char *cram(const char *challenge_64, const char *user, const char *pass)
+static char *cram(const char *challenge_64 UNUSED,
+		  const char *user UNUSED,
+		  const char *pass UNUSED)
 {
 	die("If you want to use CRAM-MD5 authenticate method, "
 	    "you have to build git-imap-send with OpenSSL library.");

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 17/22] grep: mark unused parmaeters in pcre fallbacks

When USE_LIBPCRE2 is not defined, we compile several noop fallbacks. These need to have their parameters annotated to avoid -Wunused-parameter warnings (and obviously we cannot remove the parameters, since the functions must match the non-fallback versions).

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

Show changes to grep.c +6 −4

diff --git a/grep.c b/grep.c
index 0124eb1960..fc2d0c837a 100644
--- a/grep.c
+++ b/grep.c
@@ -452,18 +452,20 @@ static void free_pcre2_pattern(struct grep_pat *p)
 	pcre2_general_context_free(p->pcre2_general_context);
 }
 #else /* !USE_LIBPCRE2 */
-static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
+static void compile_pcre2_pattern(struct grep_pat *p UNUSED,
+				  const struct grep_opt *opt UNUSED)
 {
 	die("cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE");
 }
 
-static int pcre2match(struct grep_pat *p, const char *line, const char *eol,
-		regmatch_t *match, int eflags)
+static int pcre2match(struct grep_pat *p UNUSED, const char *line UNUSED,
+		      const char *eol UNUSED, regmatch_t *match UNUSED,
+		      int eflags UNUSED)
 {
 	return 1;
 }
 
-static void free_pcre2_pattern(struct grep_pat *p)
+static void free_pcre2_pattern(struct grep_pat *p UNUSED)
 {
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 18/22] credential: mark unused parameter in urlmatch callback

Our select_all() callback does not need to actually look at its parameters, since the point is to match everything. But we need to mark its parameters to satisfy -Wunused-parameter.

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

Show changes to credential.c +2 −2

diff --git a/credential.c b/credential.c
index d664754163..18098bd35e 100644
--- a/credential.c
+++ b/credential.c
@@ -88,8 +88,8 @@ static int proto_is_http(const char *s)
 static void credential_describe(struct credential *c, struct strbuf *out);
 static void credential_format(struct credential *c, struct strbuf *out);
 
-static int select_all(const struct urlmatch_item *a,
-		      const struct urlmatch_item *b)
+static int select_all(const struct urlmatch_item *a UNUSED,
+		      const struct urlmatch_item *b UNUSED)
 {
 	return 0;
 }

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 19/22] fetch: mark unused parameter in ref_transaction callback

Since this callback is just trying to collect the set of queued tag updates, there is no need for it to look at old_oid at all. Mark it as unused to appease -Wunused-parameter.

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

Show changes to builtin/fetch.c +1 −1

diff --git a/builtin/fetch.c b/builtin/fetch.c
index eed4a7cdb6..8f93529505 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -308,7 +308,7 @@ static void clear_item(struct refname_hash_entry *item)
 
 
 static void add_already_queued_tags(const char *refname,
-				    const struct object_id *old_oid,
+				    const struct object_id *old_oid UNUSED,
 				    const struct object_id *new_oid,
 				    void *cb_data)
 {

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 20/22] bundle-uri: mark unused parameters in callbacks

The first hunk is similar to 02c3c59e62 (hashmap: mark unused callback parameters, 2022-08-19), but was added after that commit.

The other two are used with for_all_bundles_in_list(), but don't use their void data pointer.

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

Show changes to bundle-uri.c +3 −3

diff --git a/bundle-uri.c b/bundle-uri.c
index 4b5c49b93d..8492fffd2f 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -20,7 +20,7 @@ static struct {
 	{ BUNDLE_HEURISTIC_CREATIONTOKEN, "creationToken" },
 };
 
-static int compare_bundles(const void *hashmap_cmp_fn_data,
+static int compare_bundles(const void *hashmap_cmp_fn_data UNUSED,
 			   const struct hashmap_entry *he1,
 			   const struct hashmap_entry *he2,
 			   const void *id)
@@ -45,7 +45,7 @@ void init_bundle_list(struct bundle_list *list)
 }
 
 static int clear_remote_bundle_info(struct remote_bundle_info *bundle,
-				    void *data)
+				    void *data UNUSED)
 {
 	FREE_AND_NULL(bundle->id);
 	FREE_AND_NULL(bundle->uri);
@@ -779,7 +779,7 @@ static int unbundle_all_bundles(struct repository *r,
 	return 0;
 }
 
-static int unlink_bundle(struct remote_bundle_info *info, void *data)
+static int unlink_bundle(struct remote_bundle_info *info, void *data UNUSED)
 {
 	if (info->file)
 		unlink_or_warn(info->file);

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 22/22] update-ref: mark unused parameter in parser callbacks

The parsing of stdin is driven by a table of function pointers; mark unused parameters in concrete functions to avoid -Wunused-parameter warnings.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/update-ref.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Show changes to builtin/update-ref.c +7 −7

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 242102273e..c0c4e65e6f 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -311,8 +311,8 @@ static void report_ok(const char *command)
 	fflush(stdout);
 }
 
-static void parse_cmd_option(struct ref_transaction *transaction,
-			     const char *next, const char *end)
+static void parse_cmd_option(struct ref_transaction *transaction UNUSED,
+			     const char *next, const char *end UNUSED)
 {
 	const char *rest;
 	if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination)
@@ -321,16 +321,16 @@ static void parse_cmd_option(struct ref_transaction *transaction,
 		die("option unknown: %s", next);
 }
 
-static void parse_cmd_start(struct ref_transaction *transaction,
-			    const char *next, const char *end)
+static void parse_cmd_start(struct ref_transaction *transaction UNUSED,
+			    const char *next, const char *end UNUSED)
 {
 	if (*next != line_termination)
 		die("start: extra input: %s", next);
 	report_ok("start");
 }
 
 static void parse_cmd_prepare(struct ref_transaction *transaction,
-			      const char *next, const char *end)
+			      const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)
@@ -341,7 +341,7 @@ static void parse_cmd_prepare(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_abort(struct ref_transaction *transaction,
-			    const char *next, const char *end)
+			    const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)
@@ -352,7 +352,7 @@ static void parse_cmd_abort(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_commit(struct ref_transaction *transaction,
-			     const char *next, const char *end)
+			     const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)

-- 
2.42.0.505.g4c6fb48dec

Jeff King· Aug 28, 2023, 21:48 UTC · re: Jeff King · lore

[PATCH 21/22] gc: mark unused descriptors in scheduler callbacks

Each of the scheduler update callbacks gets the descriptor of the lock file, but only the crontab updater needs it. We have to retain the unused descriptors because these are dispatched from a table of function pointers, but we should mark them to silence -Wunused-parameter.

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

Show changes to builtin/gc.c +3 −3

diff --git a/builtin/gc.c b/builtin/gc.c
index 1f53b66c7b..369bd43fb2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1934,7 +1934,7 @@ static int launchctl_add_plists(void)
 	       launchctl_schedule_plist(exec_path, SCHEDULE_WEEKLY);
 }
 
-static int launchctl_update_schedule(int run_maintenance, int fd)
+static int launchctl_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return launchctl_add_plists();
@@ -2115,7 +2115,7 @@ static int schtasks_schedule_tasks(void)
 	       schtasks_schedule_task(exec_path, SCHEDULE_WEEKLY);
 }
 
-static int schtasks_update_schedule(int run_maintenance, int fd)
+static int schtasks_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return schtasks_schedule_tasks();
@@ -2556,7 +2556,7 @@ static int systemd_timer_setup_units(void)
 	return ret;
 }
 
-static int systemd_timer_update_schedule(int run_maintenance, int fd)
+static int systemd_timer_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return systemd_timer_setup_units();

-- 
2.42.0.505.g4c6fb48dec

Taylor Blau· Aug 28, 2023, 23:35 UTC · re: Jeff King · lore

Re: [PATCH 0/22] YAUPS: Yet Another Unused Parameter Series

On Mon, Aug 28, 2023 at 05:46:04PM -0400, Jeff King wrote:

Show 7 quoted lines

> I'm back with another exciting installment of -Wunused-parameter warning
> fixes. Most of these are pretty boring and obvious; the first two are
> the most interesting in terms of rationale.
>
> I promise we're closing in on the finish line here. I only have about 20
> patches left after this, at which point we should be able to turn on the
> warning by default for developer builds.

Heh ;-). It's good to see us getting dangerously close to being able to turn on -Wunused-parameter in the DEVELOPER builds.

I reviewed this series, and all looked sensible. I left a couple notes throughout for potential further cleanups that we could do on top, which might be nice to squash in.

But I wouldn't be heartbroken if you want to ignore them, either. So with or without my suggestions, this series LGTM.

Thanks, Taylor

Jeff King· Aug 29, 2023, 23:43 UTC · re: Jeff King · lore

[PATCH v2 0/22] Yet Another Unused Parameter Series

On Mon, Aug 28, 2023 at 05:46:04PM -0400, Jeff King wrote:

Show 7 quoted lines

> I'm back with another exciting installment of -Wunused-parameter warning
> fixes. Most of these are pretty boring and obvious; the first two are
> the most interesting in terms of rationale.
> 
> I promise we're closing in on the finish line here. I only have about 20
> patches left after this, at which point we should be able to turn on the
> warning by default for developer builds.

And here's a v2 based on feedback from round 1. The changes are small, so I'll let the range-diff below speak for itself.

 1:  6009723d8a !  1:  c11e9b9bd6 sequencer: use repository parameter in short_commit_name()
    @@ Commit message
         the_repository, but this pushes us one small step forward to eventually
         getting rid of those.
     
    +    Note that a few of these functions have a "rev_info" whose "repo"
    +    parameter could probably be used instead of the_repository. I'm leaving
    +    that for further cleanups, as it's not immediately obvious that
    +    revs->repo is always valid, and there's quite a bit of other possible
    +    refactoring here (even getting rid of some "struct repository" arguments
    +    in favor of revs->repo).
    +
         Signed-off-by: Jeff King <peff@peff.net>
     
      ## sequencer.c ##
    @@ sequencer.c: static int walk_revs_populate_todo(struct todo_list *todo_list,
      		subject_len = find_commit_subject(commit_buffer, &subject);
      		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
     -			short_commit_name(commit), subject_len, subject);
    -+			short_commit_name(opts->revs->repo, commit),
    ++			short_commit_name(the_repository, commit),
     +			subject_len, subject);
      		repo_unuse_commit_buffer(the_repository, commit,
      					 commit_buffer);
    @@ sequencer.c: static int make_script_with_merges(struct pretty_print_context *pp,
      			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
      				warning(_("skipped previously applied commit %s"),
     -					short_commit_name(commit));
    -+					short_commit_name(revs->repo, commit));
    ++					short_commit_name(the_repository, commit));
      			skipped_commit = 1;
      			continue;
      		}
 2:  f9068919f9 !  2:  3d0030ed7a sequencer: mark repository argument as unused
    @@ Commit message
         sequencer: mark repository argument as unused
     
         In sequencer_get_last_command(), we don't ever look at the repository
    -    parameter. It _should_ be used when calling into git_path_* functions,
    +    parameter. This is due to ed5b1ca10b (status: do not report errors in
    +    sequencer/todo, 2019-06-27), which dropped the call to parse_insn_line().
    +
    +    However, it _should_ be used when calling into git_path_* functions,
         but the one we use here is declared with the non-REPO variant of
         GIT_PATH_FUNC(), and so just uses the_repository internally.
     
 3:  cf28e8793a =  3:  deca060ede ref-filter: mark unused parameters in parser callbacks
 4:  baba1341f3 =  4:  4c15910f4e pack-bitmap: mark unused parameters in show_object callback
 5:  444d67676b =  5:  9125111e5e worktree: mark unused parameters in each_ref_fn callback
 6:  4d5fbed1aa !  6:  4e044c1005 commit-graph: mark unused data parameters in generation callbacks
    @@ Commit message
         get/set generation operations. Some callers don't need the extra void
         data pointer, which should be annotated to appease -Wunused-parameter.
     
    +    Note that we can drop the assignment of the "data" parameter in
    +    compute_generation_numbers(), as we've just shown that neither of the
    +    callbacks it uses will access it. This matches the caller in
    +    ensure_generations_valid(), which already does not bother to set "data".
    +
         Signed-off-by: Jeff King <peff@peff.net>
     
      ## commit-graph.c ##
    @@ commit-graph.c: static void compute_topological_levels(struct write_commit_graph
      {
      	struct commit_graph_data *g = commit_graph_data_at(c);
      	g->generation = t;
    +@@ commit-graph.c: static void compute_generation_numbers(struct write_commit_graph_context *ctx)
    + 		.commits = &ctx->commits,
    + 		.get_generation = get_generation_from_graph_data,
    + 		.set_generation = set_generation_v2,
    +-		.data = ctx,
    + 	};
    + 
    + 	if (ctx->report_progress)
     @@ commit-graph.c: static void compute_generation_numbers(struct write_commit_graph_context *ctx)
      }
      
 7:  b5ad1e7e6a =  7:  8441011ab3 ls-tree: mark unused parameter in callback
 8:  d0d9fb3c7e =  8:  4ee523564a stash: mark unused parameter in diff callback
 9:  eefe7daf5d =  9:  21a23b51a9 trace2: mark unused us_elapsed_absolute parameters
10:  38021bc8ce = 10:  76db35e8f1 trace2: mark unused config callback parameter
11:  a6eccfab8b = 11:  312c6bd59b test-trace2: mark unused argv/argc parameters
12:  6d971c4381 = 12:  e74362f5a8 grep: mark unused parameter in output function
13:  ac7347663c = 13:  a8525ab460 add-interactive: mark unused callback parameters
14:  d14aab5b51 = 14:  27df60c02f negotiator/noop: mark unused callback parameters
15:  2d405dc294 = 15:  4eeac4bc33 worktree: mark unused parameters in noop repair callback
16:  9c1f3b668b = 16:  d81464b1f1 imap-send: mark unused parameters with NO_OPENSSL
17:  ad63f999b9 = 17:  ea865a1077 grep: mark unused parmaeters in pcre fallbacks
18:  af414f38fc = 18:  335c42823b credential: mark unused parameter in urlmatch callback
19:  14694671eb = 19:  f06f36371b fetch: mark unused parameter in ref_transaction callback
20:  a375abd8fa = 20:  b1fe7e7e34 bundle-uri: mark unused parameters in callbacks
21:  611dc4d367 = 21:  c9e258e0fa gc: mark unused descriptors in scheduler callbacks
22:  0ec53b7a46 = 22:  7c2c8fd614 update-ref: mark unused parameter in parser callbacks

  [01/22]: sequencer: use repository parameter in short_commit_name()
  [02/22]: sequencer: mark repository argument as unused
  [03/22]: ref-filter: mark unused parameters in parser callbacks
  [04/22]: pack-bitmap: mark unused parameters in show_object callback
  [05/22]: worktree: mark unused parameters in each_ref_fn callback
  [06/22]: commit-graph: mark unused data parameters in generation callbacks
  [07/22]: ls-tree: mark unused parameter in callback
  [08/22]: stash: mark unused parameter in diff callback
  [09/22]: trace2: mark unused us_elapsed_absolute parameters
  [10/22]: trace2: mark unused config callback parameter
  [11/22]: test-trace2: mark unused argv/argc parameters
  [12/22]: grep: mark unused parameter in output function
  [13/22]: add-interactive: mark unused callback parameters
  [14/22]: negotiator/noop: mark unused callback parameters
  [15/22]: worktree: mark unused parameters in noop repair callback
  [16/22]: imap-send: mark unused parameters with NO_OPENSSL
  [17/22]: grep: mark unused parmaeters in pcre fallbacks
  [18/22]: credential: mark unused parameter in urlmatch callback
  [19/22]: fetch: mark unused parameter in ref_transaction callback
  [20/22]: bundle-uri: mark unused parameters in callbacks
  [21/22]: gc: mark unused descriptors in scheduler callbacks
  [22/22]: update-ref: mark unused parameter in parser callbacks

 add-interactive.c       |  8 ++++----
 builtin/fetch.c         |  2 +-
 builtin/gc.c            |  6 +++---
 builtin/ls-tree.c       |  3 ++-
 builtin/stash.c         |  2 +-
 builtin/update-ref.c    | 14 +++++++-------
 builtin/worktree.c      |  8 ++++----
 bundle-uri.c            |  6 +++---
 commit-graph.c          |  9 +++++----
 credential.c            |  4 ++--
 grep.c                  | 12 +++++++-----
 imap-send.c             | 10 ++++++++--
 negotiator/noop.c       | 12 +++++++-----
 pack-bitmap.c           |  5 +++--
 ref-filter.c            |  8 +++++---
 sequencer.c             | 27 ++++++++++++++-------------
 t/helper/test-trace2.c  |  6 +++---
 trace2/tr2_sysenv.c     |  3 ++-
 trace2/tr2_tgt_event.c  | 23 +++++++++++++----------
 trace2/tr2_tgt_normal.c | 20 ++++++++++++--------
 worktree.c              |  6 ++++--
 21 files changed, 110 insertions(+), 84 deletions(-)

Jeff King· Aug 29, 2023, 23:43 UTC · re: Jeff King · lore

[PATCH v2 01/22] sequencer: use repository parameter in short_commit_name()

Note that a few of these functions have a "rev_info" whose "repo" parameter could probably be used instead of the_repository. I'm leaving that for further cleanups, as it's not immediately obvious that revs->repo is always valid, and there's quite a bit of other possible refactoring here (even getting rid of some "struct repository" arguments in favor of revs->repo).

Signed-off-by: Jeff King <peff@peff.net>
---
 sequencer.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

Show changes to sequencer.c +13 −12

diff --git a/sequencer.c b/sequencer.c
index 48475d1cc6..cd5264171a 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -433,10 +433,9 @@ struct commit_message {
 	const char *message;
 };
 
-static const char *short_commit_name(struct commit *commit)
+static const char *short_commit_name(struct repository *r, struct commit *commit)
 {
-	return repo_find_unique_abbrev(the_repository, &commit->object.oid,
-				       DEFAULT_ABBREV);
+	return repo_find_unique_abbrev(r, &commit->object.oid, DEFAULT_ABBREV);
 }
 
 static int get_message(struct commit *commit, struct commit_message *out)
@@ -446,7 +445,7 @@ static int get_message(struct commit *commit, struct commit_message *out)
 
 	out->message = repo_logmsg_reencode(the_repository, commit, NULL,
 					    get_commit_output_encoding());
-	abbrev = short_commit_name(commit);
+	abbrev = short_commit_name(the_repository, commit);
 
 	subject_len = find_commit_subject(out->message, &subject);
 
@@ -2383,7 +2382,7 @@ static int do_pick_commit(struct repository *r,
 		error(command == TODO_REVERT
 		      ? _("could not revert %s... %s")
 		      : _("could not apply %s... %s"),
-		      short_commit_name(commit), msg.subject);
+		      short_commit_name(r, commit), msg.subject);
 		print_advice(r, res == 1, opts);
 		repo_rerere(r, opts->allow_rerere_auto);
 		goto leave;
@@ -3172,7 +3171,8 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
 		item->offset_in_buf = todo_list->buf.len;
 		subject_len = find_commit_subject(commit_buffer, &subject);
 		strbuf_addf(&todo_list->buf, "%s %s %.*s\n", command_string,
-			short_commit_name(commit), subject_len, subject);
+			short_commit_name(the_repository, commit),
+			subject_len, subject);
 		repo_unuse_commit_buffer(the_repository, commit,
 					 commit_buffer);
 	}
@@ -3593,7 +3593,7 @@ static int error_with_patch(struct repository *r,
 	} else if (exit_code) {
 		if (commit)
 			fprintf_ln(stderr, _("Could not apply %s... %.*s"),
-				   short_commit_name(commit), subject_len, subject);
+				   short_commit_name(r, commit), subject_len, subject);
 		else
 			/*
 			 * We don't have the hash of the parent so
@@ -4728,7 +4728,7 @@ static int pick_commits(struct repository *r,
 						term_clear_line();
 					fprintf(stderr,
 						_("Stopped at %s...  %.*s\n"),
-						short_commit_name(commit),
+						short_commit_name(r, commit),
 						item->arg_len, arg);
 				}
 				return error_with_patch(r, commit,
@@ -5564,7 +5564,7 @@ static int make_script_with_merges(struct pretty_print_context *pp,
 		if (!is_empty && (commit->object.flags & PATCHSAME)) {
 			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
 				warning(_("skipped previously applied commit %s"),
-					short_commit_name(commit));
+					short_commit_name(the_repository, commit));
 			skipped_commit = 1;
 			continue;
 		}
@@ -5800,7 +5800,7 @@ int sequencer_make_script(struct repository *r, struct strbuf *out, int argc,
 		if (!is_empty && (commit->object.flags & PATCHSAME)) {
 			if (flags & TODO_LIST_WARN_SKIPPED_CHERRY_PICKS)
 				warning(_("skipped previously applied commit %s"),
-					short_commit_name(commit));
+					short_commit_name(r, commit));
 			skipped_commit = 1;
 			continue;
 		}
@@ -5892,7 +5892,8 @@ static void todo_list_add_exec_commands(struct todo_list *todo_list,
 	todo_list->alloc = alloc;
 }
 
-static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_list,
+static void todo_list_to_strbuf(struct repository *r,
+				struct todo_list *todo_list,
 				struct strbuf *buf, int num, unsigned flags)
 {
 	struct todo_item *item;
@@ -5921,7 +5922,7 @@ static void todo_list_to_strbuf(struct repository *r, struct todo_list *todo_lis
 		/* add commit id */
 		if (item->commit) {
 			const char *oid = flags & TODO_LIST_SHORTEN_IDS ?
-					  short_commit_name(item->commit) :
+					  short_commit_name(r, item->commit) :
 					  oid_to_hex(&item->commit->object.oid);
 
 			if (item->command == TODO_FIXUP) {

-- 
2.42.0.528.g7950723a09

Phillip Wood· Aug 30, 2023, 13:24 UTC · re: Jeff King · lore

Re: [PATCH v2 01/22] sequencer: use repository parameter in short_commit_name()

On 30/08/2023 00:43, Jeff King wrote:

Show 12 quoted lines

> Instead of just using the_repository, we can take a repository parameter
> from the caller. Most of them already have one, and doing so clears up a
> few -Wunused-parameter warnings. There are still a few callers which use
> the_repository, but this pushes us one small step forward to eventually
> getting rid of those.
> 
> Note that a few of these functions have a "rev_info" whose "repo"
> parameter could probably be used instead of the_repository. I'm leaving
> that for further cleanups, as it's not immediately obvious that
> revs->repo is always valid, and there's quite a bit of other possible
> refactoring here (even getting rid of some "struct repository" arguments
> in favor of revs->repo).

I think opts->revs is only initialized when we're building to todo list for cherry-picking/reverting a sequence of commits so I the scope for removing "struct repository" arguments is pretty limited as any function that is called by "cherry-pick --continue" or rebase needs a separate repository argument. I'm pretty sure your v1 was safe but this version is much more obviously safe.

Best Wishes

Phillip

Jeff King· Aug 29, 2023, 23:44 UTC · re: Jeff King · lore

[PATCH v2 02/22] sequencer: mark repository argument as unused

In sequencer_get_last_command(), we don't ever look at the repository parameter. This is due to ed5b1ca10b (status: do not report errors in sequencer/todo, 2019-06-27), which dropped the call to parse_insn_line().

However, it _should_ be used when calling into git_path_* functions, but the one we use here is declared with the non-REPO variant of GIT_PATH_FUNC(), and so just uses the_repository internally.

So let's punt on that for now and just silence any -Wunused-parameter warning.

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

Show changes to sequencer.c +1 −1

diff --git a/sequencer.c b/sequencer.c
index cd5264171a..70696d4a65 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2649,7 +2649,7 @@ static int parse_insn_line(struct repository *r, struct todo_item *item,
 	return item->commit ? 0 : -1;
 }
 
-int sequencer_get_last_command(struct repository *r, enum replay_action *action)
+int sequencer_get_last_command(struct repository *r UNUSED, enum replay_action *action)
 {
 	const char *todo_file, *bol;
 	struct strbuf buf = STRBUF_INIT;

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 03/22] ref-filter: mark unused parameters in parser callbacks

These are similar to the cases annotated in 5fe9e1ce2f (ref-filter: mark unused callback parameters, 2023-02-24), but were added after that commit.

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

Show changes to ref-filter.c +5 −3

diff --git a/ref-filter.c b/ref-filter.c
index 1bfaf20fbf..88b021dd1d 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -549,7 +549,8 @@ static int signature_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int trailers_atom_parser(struct ref_format *format, struct used_atom *atom,
+static int trailers_atom_parser(struct ref_format *format UNUSED,
+				struct used_atom *atom,
 				const char *arg, struct strbuf *err)
 {
 	atom->u.contents.trailer_opts.no_divider = 1;
@@ -819,7 +820,7 @@ static int if_atom_parser(struct ref_format *format UNUSED,
 	return 0;
 }
 
-static int rest_atom_parser(struct ref_format *format,
+static int rest_atom_parser(struct ref_format *format UNUSED,
 			    struct used_atom *atom UNUSED,
 			    const char *arg, struct strbuf *err)
 {
@@ -828,7 +829,8 @@ static int rest_atom_parser(struct ref_format *format,
 	return 0;
 }
 
-static int ahead_behind_atom_parser(struct ref_format *format, struct used_atom *atom,
+static int ahead_behind_atom_parser(struct ref_format *format,
+				    struct used_atom *atom UNUSED,
 				    const char *arg, struct strbuf *err)
 {
 	struct string_list_item *item;

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 04/22] pack-bitmap: mark unused parameters in show_object callback

This is similar to the cases in c50dca2a18 (list-objects: mark unused callback parameters, 2023-02-24), but was added after that commit.

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

Show changes to pack-bitmap.c +3 −2

diff --git a/pack-bitmap.c b/pack-bitmap.c
index 6afc03d1e4..ca8319b87c 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -1101,8 +1101,9 @@ static void show_boundary_commit(struct commit *commit, void *_data)
 	}
 }
 
-static void show_boundary_object(struct object *object,
-				 const char *name, void *data)
+static void show_boundary_object(struct object *object UNUSED,
+				 const char *name UNUSED,
+				 void *data UNUSED)
 {
 	BUG("should not be called");
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 06/22] commit-graph: mark unused data parameters in generation callbacks

Note that we can drop the assignment of the "data" parameter in compute_generation_numbers(), as we've just shown that neither of the callbacks it uses will access it. This matches the caller in ensure_generations_valid(), which already does not bother to set "data".

Signed-off-by: Jeff King <peff@peff.net>
---
 commit-graph.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Show changes to commit-graph.c +5 −4

diff --git a/commit-graph.c b/commit-graph.c
index 0aa1640d15..e11f326097 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -1568,12 +1568,14 @@ static void compute_topological_levels(struct write_commit_graph_context *ctx)
 	stop_progress(&ctx->progress);
 }
 
-static timestamp_t get_generation_from_graph_data(struct commit *c, void *data)
+static timestamp_t get_generation_from_graph_data(struct commit *c,
+						  void *data UNUSED)
 {
 	return commit_graph_data_at(c)->generation;
 }
 
-static void set_generation_v2(struct commit *c, timestamp_t t, void *data)
+static void set_generation_v2(struct commit *c, timestamp_t t,
+			      void *data UNUSED)
 {
 	struct commit_graph_data *g = commit_graph_data_at(c);
 	g->generation = t;
@@ -1587,7 +1589,6 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 		.commits = &ctx->commits,
 		.get_generation = get_generation_from_graph_data,
 		.set_generation = set_generation_v2,
-		.data = ctx,
 	};
 
 	if (ctx->report_progress)
@@ -1616,7 +1617,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)
 }
 
 static void set_generation_in_graph_data(struct commit *c, timestamp_t t,
-					 void *data)
+					 void *data UNUSED)
 {
 	commit_graph_data_at(c)->generation = t;
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 07/22] ls-tree: mark unused parameter in callback

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/ls-tree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Show changes to builtin/ls-tree.c +2 −1

diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index f558db5f3b..209d2dc0d5 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -241,7 +241,8 @@ static int show_tree_long(const struct object_id *oid, struct strbuf *base,
 	return recurse;
 }
 
-static int show_tree_name_only(const struct object_id *oid, struct strbuf *base,
+static int show_tree_name_only(const struct object_id *oid UNUSED,
+			       struct strbuf *base,
 			       const char *pathname, unsigned mode,
 			       void *context)
 {

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 05/22] worktree: mark unused parameters in each_ref_fn callback

This is similar to the cases in 63e14ee2d6 (refs: mark unused each_ref_fn parameters, 2022-08-19), but it was added after that commit.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/worktree.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to builtin/worktree.c +4 −4

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 10db70b7ec..62b7e26f4b 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -628,10 +628,10 @@ static void print_preparing_worktree_line(int detach,
  *
  * Returns 0 on failure and non-zero on success.
  */
-static int first_valid_ref(const char *refname,
-			   const struct object_id *oid,
-			   int flags,
-			   void *cb_data)
+static int first_valid_ref(const char *refname UNUSED,
+			   const struct object_id *oid UNUSED,
+			   int flags UNUSED,
+			   void *cb_data UNUSED)
 {
 	return 1;
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 08/22] stash: mark unused parameter in diff callback

This is similar to the cases in 61bdc7c5d8 (diff: mark unused parameters in callbacks, 2022-12-13), but I missed it when making that commit.

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

Show changes to builtin/stash.c +1 −1

diff --git a/builtin/stash.c b/builtin/stash.c
index fe64cde9ce..c9365bea13 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -362,7 +362,7 @@ static int is_path_a_directory(const char *path)
 }
 
 static void add_diff_to_buf(struct diff_queue_struct *q,
-			    struct diff_options *options,
+			    struct diff_options *options UNUSED,
 			    void *data)
 {
 	int i;

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 10/22] trace2: mark unused config callback parameter

This should have been part of 783a86c142 (config: mark unused callback parameters, 2022-08-19), but was missed in that commit.

Signed-off-by: Jeff King <peff@peff.net>
---
 trace2/tr2_sysenv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Show changes to trace2/tr2_sysenv.c +2 −1

diff --git a/trace2/tr2_sysenv.c b/trace2/tr2_sysenv.c
index f26ec95ab4..d3ecac2772 100644
--- a/trace2/tr2_sysenv.c
+++ b/trace2/tr2_sysenv.c
@@ -58,7 +58,8 @@ static struct tr2_sysenv_entry tr2_sysenv_settings[] = {
 /* clang-format on */
 
 static int tr2_sysenv_cb(const char *key, const char *value,
-			 const struct config_context *ctx UNUSED, void *d)
+			 const struct config_context *ctx UNUSED,
+			 void *d UNUSED)
 {
 	int k;

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 09/22] trace2: mark unused us_elapsed_absolute parameters

Many trace2 targets ignore the absolute elapsed time parameters. However, the virtual interface needs to retain the parameter since it is used by others (e.g., the perf target).

Signed-off-by: Jeff King <peff@peff.net>
---
 trace2/tr2_tgt_event.c  | 23 +++++++++++++----------
 trace2/tr2_tgt_normal.c | 20 ++++++++++++--------
 2 files changed, 25 insertions(+), 18 deletions(-)

Show changes to 2 files +25 −18

trace2/tr2_tgt_event.c, trace2/tr2_tgt_normal.c

diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index 53091781ec..59910a1a4f 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c
@@ -335,7 +335,7 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
 }
 
 static void fn_child_start_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      const struct child_process *cmd)
 {
 	const char *event_name = "child_start";
@@ -367,7 +367,8 @@ static void fn_child_start_fl(const char *file, int line,
 }
 
 static void fn_child_exit_fl(const char *file, int line,
-			     uint64_t us_elapsed_absolute, int cid, int pid,
+			     uint64_t us_elapsed_absolute UNUSED,
+			     int cid, int pid,
 			     int code, uint64_t us_elapsed_child)
 {
 	const char *event_name = "child_exit";
@@ -388,7 +389,8 @@ static void fn_child_exit_fl(const char *file, int line,
 }
 
 static void fn_child_ready_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int cid, int pid,
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int cid, int pid,
 			      const char *ready, uint64_t us_elapsed_child)
 {
 	const char *event_name = "child_ready";
@@ -409,7 +411,7 @@ static void fn_child_ready_fl(const char *file, int line,
 }
 
 static void fn_thread_start_fl(const char *file, int line,
-			       uint64_t us_elapsed_absolute)
+			       uint64_t us_elapsed_absolute UNUSED)
 {
 	const char *event_name = "thread_start";
 	struct json_writer jw = JSON_WRITER_INIT;
@@ -423,7 +425,7 @@ static void fn_thread_start_fl(const char *file, int line,
 }
 
 static void fn_thread_exit_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      uint64_t us_elapsed_thread)
 {
 	const char *event_name = "thread_exit";
@@ -439,7 +441,8 @@ static void fn_thread_exit_fl(const char *file, int line,
 	jw_release(&jw);
 }
 
-static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
+static void fn_exec_fl(const char *file, int line,
+		       uint64_t us_elapsed_absolute UNUSED,
 		       int exec_id, const char *exe, const char **argv)
 {
 	const char *event_name = "exec";
@@ -460,8 +463,8 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
 }
 
 static void fn_exec_result_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int exec_id,
-			      int code)
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int exec_id, int code)
 {
 	const char *event_name = "exec_result";
 	struct json_writer jw = JSON_WRITER_INIT;
@@ -511,7 +514,7 @@ static void fn_repo_fl(const char *file, int line,
 }
 
 static void fn_region_enter_printf_va_fl(const char *file, int line,
-					 uint64_t us_elapsed_absolute,
+					 uint64_t us_elapsed_absolute UNUSED,
 					 const char *category,
 					 const char *label,
 					 const struct repository *repo,
@@ -538,7 +541,7 @@ static void fn_region_enter_printf_va_fl(const char *file, int line,
 }
 
 static void fn_region_leave_printf_va_fl(
-	const char *file, int line, uint64_t us_elapsed_absolute,
+	const char *file, int line, uint64_t us_elapsed_absolute UNUSED,
 	uint64_t us_elapsed_region, const char *category, const char *label,
 	const struct repository *repo, const char *fmt, va_list ap)
 {
diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index d25ea13164..38d5ebddf6 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -86,7 +86,7 @@ static void fn_version_fl(const char *file, int line)
 }
 
 static void fn_start_fl(const char *file, int line,
-			uint64_t us_elapsed_absolute, const char **argv)
+			uint64_t us_elapsed_absolute UNUSED, const char **argv)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
 
@@ -215,7 +215,7 @@ static void fn_alias_fl(const char *file, int line, const char *alias,
 }
 
 static void fn_child_start_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute,
+			      uint64_t us_elapsed_absolute UNUSED,
 			      const struct child_process *cmd)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -243,7 +243,8 @@ static void fn_child_start_fl(const char *file, int line,
 }
 
 static void fn_child_exit_fl(const char *file, int line,
-			     uint64_t us_elapsed_absolute, int cid, int pid,
+			     uint64_t us_elapsed_absolute UNUSED,
+			     int cid, int pid,
 			     int code, uint64_t us_elapsed_child)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -256,7 +257,8 @@ static void fn_child_exit_fl(const char *file, int line,
 }
 
 static void fn_child_ready_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int cid, int pid,
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int cid, int pid,
 			      const char *ready, uint64_t us_elapsed_child)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -268,7 +270,8 @@ static void fn_child_ready_fl(const char *file, int line,
 	strbuf_release(&buf_payload);
 }
 
-static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
+static void fn_exec_fl(const char *file, int line,
+		       uint64_t us_elapsed_absolute UNUSED,
 		       int exec_id, const char *exe, const char **argv)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
@@ -284,8 +287,8 @@ static void fn_exec_fl(const char *file, int line, uint64_t us_elapsed_absolute,
 }
 
 static void fn_exec_result_fl(const char *file, int line,
-			      uint64_t us_elapsed_absolute, int exec_id,
-			      int code)
+			      uint64_t us_elapsed_absolute UNUSED,
+			      int exec_id, int code)
 {
 	struct strbuf buf_payload = STRBUF_INIT;
 
@@ -321,7 +324,8 @@ static void fn_repo_fl(const char *file, int line,
 }
 
 static void fn_printf_va_fl(const char *file, int line,
-			    uint64_t us_elapsed_absolute, const char *fmt,
+			    uint64_t us_elapsed_absolute UNUSED,
+			    const char *fmt,
 			    va_list ap)
 {
 	struct strbuf buf_payload = STRBUF_INIT;

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 11/22] test-trace2: mark unused argv/argc parameters

This is similar in spirit to 126e3b3d2a (t/helper: mark unused argv/argc arguments, 2023-03-28).

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

Show changes to t/helper/test-trace2.c +3 −3

diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
index 20c7495f38..d5ca0046c8 100644
--- a/t/helper/test-trace2.c
+++ b/t/helper/test-trace2.c
@@ -45,7 +45,7 @@ static int get_i(int *p_value, const char *data)
  * [] "def_param" events for all of the "interesting" pre-defined
  * config settings.
  */
-static int ut_001return(int argc, const char **argv)
+static int ut_001return(int argc UNUSED, const char **argv)
 {
 	int rc;
 
@@ -65,7 +65,7 @@ static int ut_001return(int argc, const char **argv)
  * [] "def_param" events for all of the "interesting" pre-defined
  * config settings.
  */
-static int ut_002exit(int argc, const char **argv)
+static int ut_002exit(int argc UNUSED, const char **argv)
 {
 	int rc;
 
@@ -201,7 +201,7 @@ static int ut_006data(int argc, const char **argv)
 	return 0;
 }
 
-static int ut_007BUG(int argc, const char **argv)
+static int ut_007BUG(int argc UNUSED, const char **argv UNUSED)
 {
 	/*
 	 * Exercise BUG() to ensure that the message is printed to trace2.

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 12/22] grep: mark unused parameter in output function

This is a callback used with grep_options.output, but we don't look at the grep_opt parameter, as we're just writing the output to stdout.

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

Show changes to grep.c +1 −1

diff --git a/grep.c b/grep.c
index 0904d55b24..0124eb1960 100644
--- a/grep.c
+++ b/grep.c
@@ -17,7 +17,7 @@ static int grep_source_load(struct grep_source *gs);
 static int grep_source_is_binary(struct grep_source *gs,
 				 struct index_state *istate);
 
-static void std_output(struct grep_opt *opt, const void *buf, size_t size)
+static void std_output(struct grep_opt *opt UNUSED, const void *buf, size_t size)
 {
 	fwrite(buf, size, 1, stdout);
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 13/22] add-interactive: mark unused callback parameters

The interactive commands are dispatched from a table of abstract pointers, but not every command uses every parameter it receives. Mark the unused ones to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 add-interactive.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Show changes to add-interactive.c +4 −4

diff --git a/add-interactive.c b/add-interactive.c
index add9a1ad43..852e8e6b2f 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -1021,9 +1021,9 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps,
 	return res;
 }
 
-static int run_help(struct add_i_state *s, const struct pathspec *unused_ps,
-		    struct prefix_item_list *unused_files,
-		    struct list_and_choose_options *unused_opts)
+static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED,
+		    struct prefix_item_list *files UNUSED,
+		    struct list_and_choose_options *opts UNUSED)
 {
 	color_fprintf_ln(stdout, s->help_color, "status        - %s",
 			 _("show paths with changes"));
@@ -1074,7 +1074,7 @@ struct print_command_item_data {
 	const char *color, *reset;
 };
 
-static void print_command_item(int i, int selected,
+static void print_command_item(int i, int selected UNUSED,
 			       struct string_list_item *item,
 			       void *print_command_item_data)
 {

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 14/22] negotiator/noop: mark unused callback parameters

The noop negotiator unsurprisingly does not bother looking at any of its parameters. Mark them unused to silence -Wunused-parameter.

Signed-off-by: Jeff King <peff@peff.net>
---
 negotiator/noop.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Show changes to negotiator/noop.c +7 −5

diff --git a/negotiator/noop.c b/negotiator/noop.c
index 7b72937686..de39028ab7 100644
--- a/negotiator/noop.c
+++ b/negotiator/noop.c
@@ -3,22 +3,24 @@
 #include "../commit.h"
 #include "../fetch-negotiator.h"
 
-static void known_common(struct fetch_negotiator *n, struct commit *c)
+static void known_common(struct fetch_negotiator *n UNUSED,
+			 struct commit *c UNUSED)
 {
 	/* do nothing */
 }
 
-static void add_tip(struct fetch_negotiator *n, struct commit *c)
+static void add_tip(struct fetch_negotiator *n UNUSED,
+		    struct commit *c UNUSED)
 {
 	/* do nothing */
 }
 
-static const struct object_id *next(struct fetch_negotiator *n)
+static const struct object_id *next(struct fetch_negotiator *n UNUSED)
 {
 	return NULL;
 }
 
-static int ack(struct fetch_negotiator *n, struct commit *c)
+static int ack(struct fetch_negotiator *n UNUSED, struct commit *c UNUSED)
 {
 	/*
 	 * This negotiator does not emit any commits, so there is no commit to
@@ -28,7 +30,7 @@ static int ack(struct fetch_negotiator *n, struct commit *c)
 	return 0;
 }
 
-static void release(struct fetch_negotiator *n)
+static void release(struct fetch_negotiator *n UNUSED)
 {
 	/* nothing to release */
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 15/22] worktree: mark unused parameters in noop repair callback

The noop repair callback unsurprisingly does not look at any of its parameters. Mark them as unused to silence -Wunused-parameter.

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

Show changes to worktree.c +4 −2

diff --git a/worktree.c b/worktree.c
index b8cf29e6a1..a56a6c2a3d 100644
--- a/worktree.c
+++ b/worktree.c
@@ -581,8 +581,10 @@ static void repair_gitfile(struct worktree *wt,
 	strbuf_release(&dotgit);
 }
 
-static void repair_noop(int iserr, const char *path, const char *msg,
-			void *cb_data)
+static void repair_noop(int iserr UNUSED,
+			const char *path UNUSED,
+			const char *msg UNUSED,
+			void *cb_data UNUSED)
 {
 	/* nothing */
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 16/22] imap-send: mark unused parameters with NO_OPENSSL

Signed-off-by: Jeff King <peff@peff.net>
---
 imap-send.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Show changes to imap-send.c +8 −2

diff --git a/imap-send.c b/imap-send.c
index 06386e0b3b..996651e4f8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -206,10 +206,14 @@ static void socket_perror(const char *func, struct imap_socket *sock, int ret)
 		else
 			fprintf(stderr, "%s: unexpected EOF\n", func);
 	}
+	/* mark as used to appease -Wunused-parameter with NO_OPENSSL */
+	(void)sock;
 }
 
 #ifdef NO_OPENSSL
-static int ssl_socket_connect(struct imap_socket *sock, int use_tls_only, int verify)
+static int ssl_socket_connect(struct imap_socket *sock UNUSED,
+			      int use_tls_only UNUSED,
+			      int verify UNUSED)
 {
 	fprintf(stderr, "SSL requested but SSL support not compiled in\n");
 	return -1;
@@ -904,7 +908,9 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
 
 #else
 
-static char *cram(const char *challenge_64, const char *user, const char *pass)
+static char *cram(const char *challenge_64 UNUSED,
+		  const char *user UNUSED,
+		  const char *pass UNUSED)
 {
 	die("If you want to use CRAM-MD5 authenticate method, "
 	    "you have to build git-imap-send with OpenSSL library.");

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 17/22] grep: mark unused parmaeters in pcre fallbacks

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

Show changes to grep.c +6 −4

diff --git a/grep.c b/grep.c
index 0124eb1960..fc2d0c837a 100644
--- a/grep.c
+++ b/grep.c
@@ -452,18 +452,20 @@ static void free_pcre2_pattern(struct grep_pat *p)
 	pcre2_general_context_free(p->pcre2_general_context);
 }
 #else /* !USE_LIBPCRE2 */
-static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
+static void compile_pcre2_pattern(struct grep_pat *p UNUSED,
+				  const struct grep_opt *opt UNUSED)
 {
 	die("cannot use Perl-compatible regexes when not compiled with USE_LIBPCRE");
 }
 
-static int pcre2match(struct grep_pat *p, const char *line, const char *eol,
-		regmatch_t *match, int eflags)
+static int pcre2match(struct grep_pat *p UNUSED, const char *line UNUSED,
+		      const char *eol UNUSED, regmatch_t *match UNUSED,
+		      int eflags UNUSED)
 {
 	return 1;
 }
 
-static void free_pcre2_pattern(struct grep_pat *p)
+static void free_pcre2_pattern(struct grep_pat *p UNUSED)
 {
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 19/22] fetch: mark unused parameter in ref_transaction callback

Since this callback is just trying to collect the set of queued tag updates, there is no need for it to look at old_oid at all. Mark it as unused to appease -Wunused-parameter.

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

Show changes to builtin/fetch.c +1 −1

diff --git a/builtin/fetch.c b/builtin/fetch.c
index eed4a7cdb6..8f93529505 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -308,7 +308,7 @@ static void clear_item(struct refname_hash_entry *item)
 
 
 static void add_already_queued_tags(const char *refname,
-				    const struct object_id *old_oid,
+				    const struct object_id *old_oid UNUSED,
 				    const struct object_id *new_oid,
 				    void *cb_data)
 {

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 18/22] credential: mark unused parameter in urlmatch callback

Our select_all() callback does not need to actually look at its parameters, since the point is to match everything. But we need to mark its parameters to satisfy -Wunused-parameter.

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

Show changes to credential.c +2 −2

diff --git a/credential.c b/credential.c
index d664754163..18098bd35e 100644
--- a/credential.c
+++ b/credential.c
@@ -88,8 +88,8 @@ static int proto_is_http(const char *s)
 static void credential_describe(struct credential *c, struct strbuf *out);
 static void credential_format(struct credential *c, struct strbuf *out);
 
-static int select_all(const struct urlmatch_item *a,
-		      const struct urlmatch_item *b)
+static int select_all(const struct urlmatch_item *a UNUSED,
+		      const struct urlmatch_item *b UNUSED)
 {
 	return 0;
 }

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 21/22] gc: mark unused descriptors in scheduler callbacks

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

Show changes to builtin/gc.c +3 −3

diff --git a/builtin/gc.c b/builtin/gc.c
index 1f53b66c7b..369bd43fb2 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -1934,7 +1934,7 @@ static int launchctl_add_plists(void)
 	       launchctl_schedule_plist(exec_path, SCHEDULE_WEEKLY);
 }
 
-static int launchctl_update_schedule(int run_maintenance, int fd)
+static int launchctl_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return launchctl_add_plists();
@@ -2115,7 +2115,7 @@ static int schtasks_schedule_tasks(void)
 	       schtasks_schedule_task(exec_path, SCHEDULE_WEEKLY);
 }
 
-static int schtasks_update_schedule(int run_maintenance, int fd)
+static int schtasks_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return schtasks_schedule_tasks();
@@ -2556,7 +2556,7 @@ static int systemd_timer_setup_units(void)
 	return ret;
 }
 
-static int systemd_timer_update_schedule(int run_maintenance, int fd)
+static int systemd_timer_update_schedule(int run_maintenance, int fd UNUSED)
 {
 	if (run_maintenance)
 		return systemd_timer_setup_units();

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 20/22] bundle-uri: mark unused parameters in callbacks

The first hunk is similar to 02c3c59e62 (hashmap: mark unused callback parameters, 2022-08-19), but was added after that commit.

The other two are used with for_all_bundles_in_list(), but don't use their void data pointer.

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

Show changes to bundle-uri.c +3 −3

diff --git a/bundle-uri.c b/bundle-uri.c
index 4b5c49b93d..8492fffd2f 100644
--- a/bundle-uri.c
+++ b/bundle-uri.c
@@ -20,7 +20,7 @@ static struct {
 	{ BUNDLE_HEURISTIC_CREATIONTOKEN, "creationToken" },
 };
 
-static int compare_bundles(const void *hashmap_cmp_fn_data,
+static int compare_bundles(const void *hashmap_cmp_fn_data UNUSED,
 			   const struct hashmap_entry *he1,
 			   const struct hashmap_entry *he2,
 			   const void *id)
@@ -45,7 +45,7 @@ void init_bundle_list(struct bundle_list *list)
 }
 
 static int clear_remote_bundle_info(struct remote_bundle_info *bundle,
-				    void *data)
+				    void *data UNUSED)
 {
 	FREE_AND_NULL(bundle->id);
 	FREE_AND_NULL(bundle->uri);
@@ -779,7 +779,7 @@ static int unbundle_all_bundles(struct repository *r,
 	return 0;
 }
 
-static int unlink_bundle(struct remote_bundle_info *info, void *data)
+static int unlink_bundle(struct remote_bundle_info *info, void *data UNUSED)
 {
 	if (info->file)
 		unlink_or_warn(info->file);

-- 
2.42.0.528.g7950723a09

Jeff King· Aug 29, 2023, 23:45 UTC · re: Jeff King · lore

[PATCH v2 22/22] update-ref: mark unused parameter in parser callbacks

The parsing of stdin is driven by a table of function pointers; mark unused parameters in concrete functions to avoid -Wunused-parameter warnings.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/update-ref.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

Show changes to builtin/update-ref.c +7 −7

diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 242102273e..c0c4e65e6f 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -311,8 +311,8 @@ static void report_ok(const char *command)
 	fflush(stdout);
 }
 
-static void parse_cmd_option(struct ref_transaction *transaction,
-			     const char *next, const char *end)
+static void parse_cmd_option(struct ref_transaction *transaction UNUSED,
+			     const char *next, const char *end UNUSED)
 {
 	const char *rest;
 	if (skip_prefix(next, "no-deref", &rest) && *rest == line_termination)
@@ -321,16 +321,16 @@ static void parse_cmd_option(struct ref_transaction *transaction,
 		die("option unknown: %s", next);
 }
 
-static void parse_cmd_start(struct ref_transaction *transaction,
-			    const char *next, const char *end)
+static void parse_cmd_start(struct ref_transaction *transaction UNUSED,
+			    const char *next, const char *end UNUSED)
 {
 	if (*next != line_termination)
 		die("start: extra input: %s", next);
 	report_ok("start");
 }
 
 static void parse_cmd_prepare(struct ref_transaction *transaction,
-			      const char *next, const char *end)
+			      const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)
@@ -341,7 +341,7 @@ static void parse_cmd_prepare(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_abort(struct ref_transaction *transaction,
-			    const char *next, const char *end)
+			    const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)
@@ -352,7 +352,7 @@ static void parse_cmd_abort(struct ref_transaction *transaction,
 }
 
 static void parse_cmd_commit(struct ref_transaction *transaction,
-			     const char *next, const char *end)
+			     const char *next, const char *end UNUSED)
 {
 	struct strbuf error = STRBUF_INIT;
 	if (*next != line_termination)

-- 
2.42.0.528.g7950723a09

Junio C Hamano· Aug 30, 2023, 01:00 UTC · re: Jeff King · lore

Re: [PATCH v2 0/22] Yet Another Unused Parameter Series

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Mon, Aug 28, 2023 at 05:46:04PM -0400, Jeff King wrote:
>
>> I'm back with another exciting installment of -Wunused-parameter warning
>> fixes. Most of these are pretty boring and obvious; the first two are
>> the most interesting in terms of rationale.
>> 
>> I promise we're closing in on the finish line here. I only have about 20
>> patches left after this, at which point we should be able to turn on the
>> warning by default for developer builds.
>
> And here's a v2 based on feedback from round 1. The changes are small,
> so I'll let the range-diff below speak for itself.

The first one in this round is consistently more conservative compared to the previous round, which is very nice.

All other updates do look good, too.

Will replace.  Thanks.