[PATCH 0/7] unused argc/argv/prefix parameters

[PATCH 0/7] unused argc/argv/prefix parameters

[Jeff King]

More from my -Wunused-parameter silencing. The first one actually fixes
a bug. The second is a cleanup, and the rest are just UNUSED annotations
(but grouped into similar cases).

  [1/7]: fast-import: fix file access when run from subdir
  [2/7]: builtins: always pass prefix to parse_options()
  [3/7]: builtins: annotate always-empty prefix parameters
  [4/7]: builtins: mark unused prefix parameters
  [5/7]: mark "argv" as unused when we check argc
  [6/7]: t/helper: mark unused argv/argc arguments
  [7/7]: parse-options: drop parse_opt_unknown_cb()

 builtin.h                            | 10 ++++++++++
 builtin/check-ref-format.c           |  2 ++
 builtin/credential.c                 |  2 +-
 builtin/diff.c                       |  6 +++---
 builtin/fast-import.c                | 10 ++++++++--
 builtin/fetch-pack.c                 |  2 +-
 builtin/fsmonitor--daemon.c          |  2 +-
 builtin/get-tar-commit-id.c          |  4 +++-
 builtin/mailsplit.c                  |  2 ++
 builtin/merge-index.c                |  2 +-
 builtin/merge-ours.c                 |  2 +-
 builtin/merge-recursive.c            |  2 +-
 builtin/mktag.c                      |  2 +-
 builtin/pack-redundant.c             |  2 +-
 builtin/remote-ext.c                 |  2 ++
 builtin/remote-fd.c                  |  2 ++
 builtin/revert.c                     |  9 +++++----
 builtin/stash.c                      |  2 +-
 builtin/submodule--helper.c          |  2 +-
 builtin/unpack-file.c                |  2 +-
 builtin/unpack-objects.c             |  2 +-
 builtin/upload-archive.c             |  2 ++
 builtin/var.c                        |  2 +-
 parse-options-cb.c                   | 15 --------------
 parse-options.h                      |  3 ---
 scalar.c                             |  2 +-
 t/helper/test-ctype.c                |  2 +-
 t/helper/test-date.c                 |  2 +-
 t/helper/test-drop-caches.c          |  2 +-
 t/helper/test-dump-cache-tree.c      |  2 +-
 t/helper/test-dump-fsmonitor.c       |  2 +-
 t/helper/test-dump-split-index.c     |  2 +-
 t/helper/test-dump-untracked-cache.c |  2 +-
 t/helper/test-example-decorate.c     |  2 +-
 t/helper/test-fsmonitor-client.c     |  2 +-
 t/helper/test-hexdump.c              |  2 +-
 t/helper/test-index-version.c        |  2 +-
 t/helper/test-match-trees.c          |  2 +-
 t/helper/test-oid-array.c            |  2 +-
 t/helper/test-oidmap.c               |  2 +-
 t/helper/test-oidtree.c              |  4 ++--
 t/helper/test-online-cpus.c          |  2 +-
 t/helper/test-parse-options.c        |  4 ++--
 t/helper/test-prio-queue.c           |  2 +-
 t/helper/test-read-graph.c           |  2 +-
 t/helper/test-ref-store.c            |  5 +++--
 t/helper/test-scrap-cache-tree.c     |  2 +-
 t/helper/test-sigchain.c             |  2 +-
 t/helper/test-strcmp-offset.c        |  2 +-
 t/helper/test-submodule-config.c     |  2 +-
 t/helper/test-submodule.c            |  2 +-
 t/helper/test-trace2.c               |  6 +++---
 t/helper/test-xml-encode.c           |  2 +-
 t/t9304-fast-import-marks.sh         | 29 ++++++++++++++++++++++++++++
 54 files changed, 115 insertions(+), 74 deletions(-)

[PATCH 1/7] fast-import: fix file access when run from subdir

[Jeff King]

In cmd_fast_import(), we ignore the "prefix" argument entirely, even
though it tells us how we may have changed directory to the root of the
repository earlier in the process. Which means that if you run it from a
subdir and point to paths in the filesystem, like:

  cd subdir
  git fast-import --import-marks=foo <dump

then it will look for "foo" in the root of the repository, not the
current directory ("subdir/") which the user would have expected.

We can fix this by recording the prefix and using it as appropriate
whenever we open a file for reading or writing. I found each of these by
looking for cases where we call fopen() within fast-import.c, so this
should cover all cases. The new test triggers each one, as well as
making sure we don't accidentally apply the prefix when --relative-marks
is in use (since that option interprets some paths as relative to a
specific directory).

Signed-off-by: Jeff King <peff@peff.net>
---
It's curious that several of the file-writing options do not respect the
"relative-marks" feature. I'm not sure if that is a bug, but if so, it
is orthogonal to this patch (and arguably fixing it now may create more
confusion than it's worth).

I also wondered if this bug was introduced when we switched fast-import
to a builtin. But nope, before that it did call setup_git_directory()
itself, but ignored the prefix value which was returned.

 builtin/fast-import.c        | 10 ++++++++--
 t/t9304-fast-import-marks.sh | 29 +++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index f7548ff4a35..19427e05f1c 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -176,6 +176,7 @@ static FILE *pack_edges;
 static unsigned int show_stats = 1;
 static int global_argc;
 static const char **global_argv;
+static const char *global_prefix;
 
 /* Memory pools */
 static struct mem_pool fi_mem_pool = {
@@ -3246,7 +3247,7 @@ static void parse_alias(void)
 static char* make_fast_import_path(const char *path)
 {
 	if (!relative_marks_paths || is_absolute_path(path))
-		return xstrdup(path);
+		return prefix_filename(global_prefix, path);
 	return git_pathdup("info/fast-import/%s", path);
 }
 
@@ -3317,9 +3318,11 @@ static void option_cat_blob_fd(const char *fd)
 
 static void option_export_pack_edges(const char *edges)
 {
+	char *fn = prefix_filename(global_prefix, edges);
 	if (pack_edges)
 		fclose(pack_edges);
-	pack_edges = xfopen(edges, "a");
+	pack_edges = xfopen(fn, "a");
+	free(fn);
 }
 
 static void option_rewrite_submodules(const char *arg, struct string_list *list)
@@ -3334,11 +3337,13 @@ static void option_rewrite_submodules(const char *arg, struct string_list *list)
 	f++;
 	CALLOC_ARRAY(ms, 1);
 
+	f = prefix_filename(global_prefix, f);
 	fp = fopen(f, "r");
 	if (!fp)
 		die_errno("cannot read '%s'", f);
 	read_mark_file(&ms, fp, insert_oid_entry);
 	fclose(fp);
+	free(f);
 
 	string_list_insert(list, s)->util = ms;
 }
@@ -3552,6 +3557,7 @@ int cmd_fast_import(int argc, const char **argv, const char *prefix)
 
 	global_argc = argc;
 	global_argv = argv;
+	global_prefix = prefix;
 
 	rc_free = mem_pool_alloc(&fi_mem_pool, cmd_save * sizeof(*rc_free));
 	for (i = 0; i < (cmd_save - 1); i++)
diff --git a/t/t9304-fast-import-marks.sh b/t/t9304-fast-import-marks.sh
index a98ef032d94..410a871c52a 100755
--- a/t/t9304-fast-import-marks.sh
+++ b/t/t9304-fast-import-marks.sh
@@ -49,4 +49,33 @@ test_expect_success 'import with submodule mapping' '
 	test_cmp expect actual
 '
 
+test_expect_success 'paths adjusted for relative subdir' '
+	git init deep-dst &&
+	mkdir deep-dst/subdir &&
+	>deep-dst/subdir/empty-marks &&
+	git -C deep-dst/subdir fast-import \
+		--rewrite-submodules-from=sub:../../from \
+		--rewrite-submodules-to=sub:../../to \
+		--import-marks=empty-marks \
+		--export-marks=exported-marks \
+		--export-pack-edges=exported-edges \
+		<dump &&
+	# we do not bother checking resulting repo; we just care that nothing
+	# complained about failing to open files for reading, and that files
+	# for writing were created in the expected spot
+	test_path_is_file deep-dst/subdir/exported-marks &&
+	test_path_is_file deep-dst/subdir/exported-edges
+'
+
+test_expect_success 'relative marks are not affected by subdir' '
+	git init deep-relative &&
+	mkdir deep-relative/subdir &&
+	git -C deep-relative/subdir fast-import \
+		--relative-marks \
+		--export-marks=exported-marks \
+		<dump &&
+	test_path_is_missing deep-relative/subdir/exported-marks &&
+	test_path_is_file deep-relative/.git/info/fast-import/exported-marks
+'
+
 test_done
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 2/7] builtins: always pass prefix to parse_options()

[Jeff King]

Our builtins receive a "prefix" argument as part of their cmd_foo()
function. We should always pass this to parse_options() if we're calling
it, as it may be used for OPT_FILENAME() options.

In the cases here, there's no option that would use it, so we're not
fixing any bug. This is just future-proofing and setting a good example
(plus quelling some -Wunused-parameter warnings).

Note in the case of revert/cherry-pick, that we plumb the prefix through
to run_sequencer(), as those builtins are just thin wrappers around it.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/mktag.c  | 2 +-
 builtin/revert.c | 9 +++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/builtin/mktag.c b/builtin/mktag.c
index 42c2457c705..967a4442dee 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -81,7 +81,7 @@ int cmd_mktag(int argc, const char **argv, const char *prefix)
 	int tagged_type;
 	struct object_id result;
 
-	argc = parse_options(argc, argv, NULL,
+	argc = parse_options(argc, argv, prefix,
 			     builtin_mktag_options,
 			     builtin_mktag_usage, 0);
 
diff --git a/builtin/revert.c b/builtin/revert.c
index 62986a7b1b0..287721fd37b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -94,7 +94,8 @@ static void verify_opt_compatible(const char *me, const char *base_opt, ...)
 		die(_("%s: %s cannot be used with %s"), me, this_opt, base_opt);
 }
 
-static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
+static int run_sequencer(int argc, const char **argv, const char *prefix,
+			 struct replay_opts *opts)
 {
 	const char * const * usage_str = revert_or_cherry_pick_usage(opts);
 	const char *me = action_name(opts);
@@ -141,7 +142,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
 		options = parse_options_concat(options, cp_extra);
 	}
 
-	argc = parse_options(argc, argv, NULL, options, usage_str,
+	argc = parse_options(argc, argv, prefix, options, usage_str,
 			PARSE_OPT_KEEP_ARGV0 |
 			PARSE_OPT_KEEP_UNKNOWN_OPT);
 
@@ -246,7 +247,7 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 
 	opts.action = REPLAY_REVERT;
 	sequencer_init_config(&opts);
-	res = run_sequencer(argc, argv, &opts);
+	res = run_sequencer(argc, argv, prefix, &opts);
 	if (res < 0)
 		die(_("revert failed"));
 	replay_opts_release(&opts);
@@ -260,7 +261,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 
 	opts.action = REPLAY_PICK;
 	sequencer_init_config(&opts);
-	res = run_sequencer(argc, argv, &opts);
+	res = run_sequencer(argc, argv, prefix, &opts);
 	if (res < 0)
 		die(_("cherry-pick failed"));
 	replay_opts_release(&opts);
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 3/7] builtins: annotate always-empty prefix parameters

[Jeff King]

It's usually a bad idea for a builtin's cmd_foo() to ignore the "prefix"
argument it gets, as it needs to prepend that string when accessing any
paths given by the user.

But if a builtin does not ask for the git wrapper to run repository
setup (via the RUN_SETUP or RUN_SETUP_GENTLY flags), then we know the
prefix will always be NULL (it is adjusting for the chdir() done during
repo setup, but there cannot be one if we did not set up the repo). In
those cases it's OK to ignore "prefix", but it's worth annotating for a
few reasons:

  1. It serves as documentation to somebody reading the code about what
     we expect.

  2. If the flags in git.c ever change, the run-time assertion may help
     detect the problem (though only if the command is run from a
     subdirectory of the repository).

  3. It notes to the compiler that we are OK ignoring "prefix". In
     particular, this silences -Wunused-parameter. It _could_ also help
     the compiler generate better code (because it will know the prefix
     is NULL), but in practice this is quite unlikely to matter.

Note that I've only added this annotation to commands which triggered
-Wunused-parameter. It would be correct to add it to any builtin which
doesn't ask for RUN_SETUP, but most of the rest of them do the sensible
thing with "prefix" by passing it to parse_options(). So they're much
more likely to just work if they ever switched to RUN_SETUP, and aren't
worth annotating.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin.h                   | 10 ++++++++++
 builtin/check-ref-format.c  |  2 ++
 builtin/get-tar-commit-id.c |  2 ++
 builtin/mailsplit.c         |  2 ++
 builtin/remote-ext.c        |  2 ++
 builtin/remote-fd.c         |  2 ++
 builtin/upload-archive.c    |  2 ++
 7 files changed, 22 insertions(+)

diff --git a/builtin.h b/builtin.h
index 46cc7897898..cb0db676814 100644
--- a/builtin.h
+++ b/builtin.h
@@ -107,6 +107,16 @@ void setup_auto_pager(const char *cmd, int def);
 
 int is_builtin(const char *s);
 
+/*
+ * Builtins which do not use RUN_SETUP should never see
+ * a prefix that is not empty; use this to protect downstream
+ * code which is not prepared to call prefix_filename(), etc.
+ */
+#define BUG_ON_NON_EMPTY_PREFIX(prefix) do { \
+	if ((prefix)) \
+		BUG("unexpected prefix in builtin: %s", (prefix)); \
+} while (0)
+
 int cmd_add(int argc, const char **argv, const char *prefix);
 int cmd_am(int argc, const char **argv, const char *prefix);
 int cmd_annotate(int argc, const char **argv, const char *prefix);
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index fd0e5f86832..462eefe1023 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -60,6 +60,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 	char *to_free = NULL;
 	int ret = 1;
 
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(builtin_check_ref_format_usage);
 
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 491af9202dc..8f8f2ac3e68 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -24,6 +24,8 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix)
 	long len;
 	char *end;
 
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	if (argc != 1)
 		usage(builtin_get_tar_commit_id_usage);
 
diff --git a/builtin/mailsplit.c b/builtin/mailsplit.c
index 73509f651bd..91e93f0c777 100644
--- a/builtin/mailsplit.c
+++ b/builtin/mailsplit.c
@@ -277,6 +277,8 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
 	const char **argp;
 	static const char *stdin_only[] = { "-", NULL };
 
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	for (argp = argv+1; *argp; argp++) {
 		const char *arg = *argp;
 
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index ee338bf440c..282782eccdd 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -197,6 +197,8 @@ static int command_loop(const char *child)
 
 int cmd_remote_ext(int argc, const char **argv, const char *prefix)
 {
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	if (argc != 3)
 		usage(usage_msg);
 
diff --git a/builtin/remote-fd.c b/builtin/remote-fd.c
index b2a3980b1d5..9020fab9c58 100644
--- a/builtin/remote-fd.c
+++ b/builtin/remote-fd.c
@@ -59,6 +59,8 @@ int cmd_remote_fd(int argc, const char **argv, const char *prefix)
 	int output_fd = -1;
 	char *end;
 
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	if (argc != 3)
 		usage(usage_msg);
 
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 945ee2b4126..7f9320ac6d0 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -79,6 +79,8 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 {
 	struct child_process writer = CHILD_PROCESS_INIT;
 
+	BUG_ON_NON_EMPTY_PREFIX(prefix);
+
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(upload_archive_usage);
 
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 4/7] builtins: mark unused prefix parameters

[Jeff King]

All builtins receive a "prefix" parameter, but it is only useful if they
need to adjust filenames given by the user on the command line. For
builtins that do not even call parse_options(), they often don't look at
the prefix at all, and -Wunused-parameter complains.

Let's annotate those to silence the compiler warning. I gave a quick
scan of each of these cases, and it seems like they don't have anything
they _should_ be using the prefix for (i.e., there is no hidden bug that
we are missing). The only questionable cases I saw were:

  - in git-unpack-file, we create a tempfile which will always be at the
    root of the repository, even if the command is run from a subdir.
    Arguably this should be created in the subdir from which we're run
    (as we report the path only as a relative name). However, nobody has
    complained, and I'm hesitant to change something that is deep
    plumbing going back to April 2005 (though I think within our
    scripts, the sole caller in git-merge-one-file would be OK, as it
    moves to the toplevel itself).

  - in fetch-pack, local-filesystem remotes are taken as relative to the
    project root, not the current directory. So:

       git init server.git
       [...put stuff in server.git...]
       git init client.git
       cd client.git
       mkdir subdir
       cd subdir
       git fetch-pack ../../server.git ...

    won't work, as we quietly move to the top of the repository before
    interpreting the path (so "../server.git" would work). This is
    weird, but again, nobody has complained and this is how it has
    always worked. And this is how "git fetch" works, too. Plus it
    raises questions about how a configured remote like:

      git config remote.origin.url ../server.git

    should behave. I can certainly come up with a reasonable set of
    behavior, but it may not be worth stirring up complications in a
    plumbing tool.

So I've left the behavior untouched in both of those cases. If anybody
really wants to revisit them, it's easy enough to drop the UNUSED
marker. This commit is just about removing them as obstacles to turning
on -Wunused-parameter all the time.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/credential.c        | 2 +-
 builtin/fetch-pack.c        | 2 +-
 builtin/fsmonitor--daemon.c | 2 +-
 builtin/merge-index.c       | 2 +-
 builtin/merge-ours.c        | 2 +-
 builtin/merge-recursive.c   | 2 +-
 builtin/pack-redundant.c    | 2 +-
 builtin/stash.c             | 2 +-
 builtin/submodule--helper.c | 2 +-
 builtin/unpack-file.c       | 2 +-
 builtin/unpack-objects.c    | 2 +-
 builtin/var.c               | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/credential.c b/builtin/credential.c
index d7b304fa084..70107529876 100644
--- a/builtin/credential.c
+++ b/builtin/credential.c
@@ -6,7 +6,7 @@
 static const char usage_msg[] =
 	"git credential (fill|approve|reject)";
 
-int cmd_credential(int argc, const char **argv, const char *prefix)
+int cmd_credential(int argc, const char **argv, const char *prefix UNUSED)
 {
 	const char *op;
 	struct credential c = CREDENTIAL_INIT;
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index d1a4306da3a..0d75e474f02 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -42,7 +42,7 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
 	(*sought)[*nr - 1] = ref;
 }
 
-int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
+int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i, ret;
 	struct ref *ref = NULL;
diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c
index cae804a1908..3d4f2ae1d0c 100644
--- a/builtin/fsmonitor--daemon.c
+++ b/builtin/fsmonitor--daemon.c
@@ -1575,7 +1575,7 @@ int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
 }
 
 #else
-int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix)
+int cmd_fsmonitor__daemon(int argc, const char **argv, const char *prefix UNUSED)
 {
 	struct option options[] = {
 		OPT_END()
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index c875f5d37ee..b747b4ed983 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -71,7 +71,7 @@ static void merge_all(void)
 	}
 }
 
-int cmd_merge_index(int argc, const char **argv, const char *prefix)
+int cmd_merge_index(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i, force_file = 0;
 
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index 284eb486098..c2e519301e9 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -14,7 +14,7 @@
 static const char builtin_merge_ours_usage[] =
 	"git merge-ours <base>... -- HEAD <remote>...";
 
-int cmd_merge_ours(int argc, const char **argv, const char *prefix)
+int cmd_merge_ours(int argc, const char **argv, const char *prefix UNUSED)
 {
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(builtin_merge_ours_usage);
diff --git a/builtin/merge-recursive.c b/builtin/merge-recursive.c
index b9acbf5d342..0a50d0a0ae2 100644
--- a/builtin/merge-recursive.c
+++ b/builtin/merge-recursive.c
@@ -20,7 +20,7 @@ static char *better_branch_name(const char *branch)
 	return xstrdup(name ? name : branch);
 }
 
-int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
+int cmd_merge_recursive(int argc, const char **argv, const char *prefix UNUSED)
 {
 	const struct object_id *bases[21];
 	unsigned bases_count = 0;
diff --git a/builtin/pack-redundant.c b/builtin/pack-redundant.c
index 82115c5808c..104d10877b4 100644
--- a/builtin/pack-redundant.c
+++ b/builtin/pack-redundant.c
@@ -558,7 +558,7 @@ static void load_all(void)
 	}
 }
 
-int cmd_pack_redundant(int argc, const char **argv, const char *prefix)
+int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i;
 	int i_still_use_this = 0;
diff --git a/builtin/stash.c b/builtin/stash.c
index 6a12fed2713..5f327435b53 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1466,7 +1466,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
 	return ret;
 }
 
-static int create_stash(int argc, const char **argv, const char *prefix)
+static int create_stash(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int ret;
 	struct strbuf stash_msg_buf = STRBUF_INIT;
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d05d1a84623..65a053261a9 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2766,7 +2766,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
 	return ret;
 }
 
-static int push_check(int argc, const char **argv, const char *prefix)
+static int push_check(int argc, const char **argv, const char *prefix UNUSED)
 {
 	struct remote *remote;
 	const char *superproject_head;
diff --git a/builtin/unpack-file.c b/builtin/unpack-file.c
index e9b105a5397..374d980c910 100644
--- a/builtin/unpack-file.c
+++ b/builtin/unpack-file.c
@@ -24,7 +24,7 @@ static char *create_temp_file(struct object_id *oid)
 	return path;
 }
 
-int cmd_unpack_file(int argc, const char **argv, const char *prefix)
+int cmd_unpack_file(int argc, const char **argv, const char *prefix UNUSED)
 {
 	struct object_id oid;
 
diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
index 1908dcfcffb..3d8510ccf81 100644
--- a/builtin/unpack-objects.c
+++ b/builtin/unpack-objects.c
@@ -600,7 +600,7 @@ static void unpack_all(void)
 		die("unresolved deltas left after unpacking");
 }
 
-int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
+int cmd_unpack_objects(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i;
 	struct object_id oid;
diff --git a/builtin/var.c b/builtin/var.c
index d9943be9afd..acb988d2d56 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -78,7 +78,7 @@ static int show_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-int cmd_var(int argc, const char **argv, const char *prefix)
+int cmd_var(int argc, const char **argv, const char *prefix UNUSED)
 {
 	const struct git_var *git_var;
 	const char *val;
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 5/7] mark "argv" as unused when we check argc

[Jeff King]

A few commands don't take any options at all, and confirm this by
checking argc. After that they have no need to look at argv, but we're
still stuck with it by convention. Let's annotate these cases so that
the compiler doesn't complain with -Wunused-parameter.

Note that in scalar and get-tar-commit-id, we're forced to keep argv by
calling convention (the functions must match cmd_main() and builtin
cmd_foo() conventions, respectively). In diff, these are subcommand
modes that we call individually, so we _could_ just drop the argv
parameters entirely. But it's weird to pass argc without argv, and it
implies that the caller knows that the subcommands aren't interested in
further arguments. It's less confusing to just keep them and silence the
compiler warning.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/diff.c              | 6 +++---
 builtin/get-tar-commit-id.c | 2 +-
 scalar.c                    | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 26f1e532c66..bc9da7bcb60 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -74,7 +74,7 @@ static void stuff_change(struct diff_options *opt,
 }
 
 static int builtin_diff_b_f(struct rev_info *revs,
-			    int argc, const char **argv,
+			    int argc, const char **argv UNUSED,
 			    struct object_array_entry **blob)
 {
 	/* Blob vs file in the working tree*/
@@ -109,7 +109,7 @@ static int builtin_diff_b_f(struct rev_info *revs,
 }
 
 static int builtin_diff_blobs(struct rev_info *revs,
-			      int argc, const char **argv,
+			      int argc, const char **argv UNUSED,
 			      struct object_array_entry **blob)
 {
 	const unsigned mode = canon_mode(S_IFREG | 0644);
@@ -209,7 +209,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 }
 
 static int builtin_diff_combined(struct rev_info *revs,
-				 int argc, const char **argv,
+				 int argc, const char **argv UNUSED,
 				 struct object_array_entry *ent,
 				 int ents, int first_non_parent)
 {
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index 8f8f2ac3e68..4324d39fb49 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -14,7 +14,7 @@ static const char builtin_get_tar_commit_id_usage[] =
 #define RECORDSIZE	(512)
 #define HEADERSIZE (2 * RECORDSIZE)
 
-int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix)
+int cmd_get_tar_commit_id(int argc, const char **argv UNUSED, const char *prefix)
 {
 	char buffer[HEADERSIZE];
 	struct ustar_header *header = (struct ustar_header *)buffer;
diff --git a/scalar.c b/scalar.c
index ca19b95ce46..c7b8c16c8ee 100644
--- a/scalar.c
+++ b/scalar.c
@@ -563,7 +563,7 @@ static int cmd_diagnose(int argc, const char **argv)
 	return res;
 }
 
-static int cmd_list(int argc, const char **argv)
+static int cmd_list(int argc, const char **argv UNUSED)
 {
 	if (argc != 1)
 		die(_("`scalar list` does not take arguments"));
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 6/7] t/helper: mark unused argv/argc arguments

[Jeff King]

Many test helper programs do not bother to look at argc or argv, because
they don't take any options. In a user-facing program, it's a good idea
to check for unexpected arguments and complain. But for a test helper,
it's not worth the trouble to enforce this.

But we do want to tell the compiler we're OK with ignoring them, to
silence -Wunused-parameter (and obviously we can't get rid of them,
since we have to conform to the usual cmd__foo() interface).

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-ctype.c                | 2 +-
 t/helper/test-date.c                 | 2 +-
 t/helper/test-drop-caches.c          | 2 +-
 t/helper/test-dump-cache-tree.c      | 2 +-
 t/helper/test-dump-fsmonitor.c       | 2 +-
 t/helper/test-dump-split-index.c     | 2 +-
 t/helper/test-dump-untracked-cache.c | 2 +-
 t/helper/test-example-decorate.c     | 2 +-
 t/helper/test-fsmonitor-client.c     | 2 +-
 t/helper/test-hexdump.c              | 2 +-
 t/helper/test-index-version.c        | 2 +-
 t/helper/test-match-trees.c          | 2 +-
 t/helper/test-oid-array.c            | 2 +-
 t/helper/test-oidmap.c               | 2 +-
 t/helper/test-oidtree.c              | 4 ++--
 t/helper/test-online-cpus.c          | 2 +-
 t/helper/test-parse-options.c        | 4 ++--
 t/helper/test-prio-queue.c           | 2 +-
 t/helper/test-read-graph.c           | 2 +-
 t/helper/test-ref-store.c            | 5 +++--
 t/helper/test-scrap-cache-tree.c     | 2 +-
 t/helper/test-sigchain.c             | 2 +-
 t/helper/test-strcmp-offset.c        | 2 +-
 t/helper/test-submodule-config.c     | 2 +-
 t/helper/test-submodule.c            | 2 +-
 t/helper/test-trace2.c               | 6 +++---
 t/helper/test-xml-encode.c           | 2 +-
 27 files changed, 33 insertions(+), 32 deletions(-)

diff --git a/t/helper/test-ctype.c b/t/helper/test-ctype.c
index 534ca66441c..71a1a5c9b04 100644
--- a/t/helper/test-ctype.c
+++ b/t/helper/test-ctype.c
@@ -47,7 +47,7 @@ static int is_in(const char *s, int ch)
 	"\x10\x11\x12\x13\x14\x15\x16\x17\x18\x19\x1a\x1b\x1c\x1d\x1e\x1f" \
 	"\x7f"
 
-int cmd__ctype(int argc, const char **argv)
+int cmd__ctype(int argc UNUSED, const char **argv UNUSED)
 {
 	TEST_CLASS(isdigit, DIGIT);
 	TEST_CLASS(isspace, " \n\r\t");
diff --git a/t/helper/test-date.c b/t/helper/test-date.c
index 45951b1df87..cd6a6df7023 100644
--- a/t/helper/test-date.c
+++ b/t/helper/test-date.c
@@ -104,7 +104,7 @@ static void getnanos(const char **argv)
 	printf("%lf\n", seconds);
 }
 
-int cmd__date(int argc, const char **argv)
+int cmd__date(int argc UNUSED, const char **argv)
 {
 	const char *x;
 
diff --git a/t/helper/test-drop-caches.c b/t/helper/test-drop-caches.c
index e37396dd9c2..73e551cfc22 100644
--- a/t/helper/test-drop-caches.c
+++ b/t/helper/test-drop-caches.c
@@ -155,7 +155,7 @@ static int cmd_dropcaches(void)
 
 #endif
 
-int cmd__drop_caches(int argc, const char **argv)
+int cmd__drop_caches(int argc UNUSED, const char **argv UNUSED)
 {
 	cmd_sync();
 	return cmd_dropcaches();
diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c
index 92dfc1aa8c4..ba1c5ce7175 100644
--- a/t/helper/test-dump-cache-tree.c
+++ b/t/helper/test-dump-cache-tree.c
@@ -57,7 +57,7 @@ static int dump_cache_tree(struct cache_tree *it,
 	return errs;
 }
 
-int cmd__dump_cache_tree(int ac, const char **av)
+int cmd__dump_cache_tree(int ac UNUSED, const char **av UNUSED)
 {
 	struct index_state istate;
 	struct cache_tree *another = cache_tree();
diff --git a/t/helper/test-dump-fsmonitor.c b/t/helper/test-dump-fsmonitor.c
index 975f0ac8905..3d2fb92ade8 100644
--- a/t/helper/test-dump-fsmonitor.c
+++ b/t/helper/test-dump-fsmonitor.c
@@ -1,7 +1,7 @@
 #include "test-tool.h"
 #include "cache.h"
 
-int cmd__dump_fsmonitor(int ac, const char **av)
+int cmd__dump_fsmonitor(int ac UNUSED, const char **av UNUSED)
 {
 	struct index_state *istate = the_repository->index;
 	int i;
diff --git a/t/helper/test-dump-split-index.c b/t/helper/test-dump-split-index.c
index 813d0a38fae..4fabbdea496 100644
--- a/t/helper/test-dump-split-index.c
+++ b/t/helper/test-dump-split-index.c
@@ -10,7 +10,7 @@ static void show_bit(size_t pos, void *data)
 	printf(" %d", (int)pos);
 }
 
-int cmd__dump_split_index(int ac, const char **av)
+int cmd__dump_split_index(int ac UNUSED, const char **av)
 {
 	struct split_index *si;
 	int i;
diff --git a/t/helper/test-dump-untracked-cache.c b/t/helper/test-dump-untracked-cache.c
index af953fabe87..bc1ef8d4dc0 100644
--- a/t/helper/test-dump-untracked-cache.c
+++ b/t/helper/test-dump-untracked-cache.c
@@ -41,7 +41,7 @@ static void dump(struct untracked_cache_dir *ucd, struct strbuf *base)
 	strbuf_setlen(base, len);
 }
 
-int cmd__dump_untracked_cache(int ac, const char **av)
+int cmd__dump_untracked_cache(int ac UNUSED, const char **av UNUSED)
 {
 	struct untracked_cache *uc;
 	struct strbuf base = STRBUF_INIT;
diff --git a/t/helper/test-example-decorate.c b/t/helper/test-example-decorate.c
index 7c7fc8efc13..2cf302ffcb3 100644
--- a/t/helper/test-example-decorate.c
+++ b/t/helper/test-example-decorate.c
@@ -3,7 +3,7 @@
 #include "object.h"
 #include "decorate.h"
 
-int cmd__example_decorate(int argc, const char **argv)
+int cmd__example_decorate(int argc UNUSED, const char **argv UNUSED)
 {
 	struct decoration n;
 	struct object_id one_oid = { {1} };
diff --git a/t/helper/test-fsmonitor-client.c b/t/helper/test-fsmonitor-client.c
index 54a4856c48c..0a1e492e5bc 100644
--- a/t/helper/test-fsmonitor-client.c
+++ b/t/helper/test-fsmonitor-client.c
@@ -11,7 +11,7 @@
 #include "trace2.h"
 
 #ifndef HAVE_FSMONITOR_DAEMON_BACKEND
-int cmd__fsmonitor_client(int argc, const char **argv)
+int cmd__fsmonitor_client(int argc UNUSED, const char **argv UNUSED)
 {
 	die("fsmonitor--daemon not available on this platform");
 }
diff --git a/t/helper/test-hexdump.c b/t/helper/test-hexdump.c
index 811e89c1bcb..05f55eca21a 100644
--- a/t/helper/test-hexdump.c
+++ b/t/helper/test-hexdump.c
@@ -4,7 +4,7 @@
 /*
  * Read stdin and print a hexdump to stdout.
  */
-int cmd__hexdump(int argc, const char **argv)
+int cmd__hexdump(int argc UNUSED, const char **argv UNUSED)
 {
 	char buf[1024];
 	ssize_t i, len;
diff --git a/t/helper/test-index-version.c b/t/helper/test-index-version.c
index fcd10968cc1..a06c45c1f84 100644
--- a/t/helper/test-index-version.c
+++ b/t/helper/test-index-version.c
@@ -1,7 +1,7 @@
 #include "test-tool.h"
 #include "cache.h"
 
-int cmd__index_version(int argc, const char **argv)
+int cmd__index_version(int argc UNUSED, const char **argv UNUSED)
 {
 	struct cache_header hdr;
 	int version;
diff --git a/t/helper/test-match-trees.c b/t/helper/test-match-trees.c
index 04bc2563f3e..184aa1a0870 100644
--- a/t/helper/test-match-trees.c
+++ b/t/helper/test-match-trees.c
@@ -3,7 +3,7 @@
 #include "hex.h"
 #include "tree.h"
 
-int cmd__match_trees(int ac, const char **av)
+int cmd__match_trees(int ac UNUSED, const char **av)
 {
 	struct object_id hash1, hash2, shifted;
 	struct tree *one, *two;
diff --git a/t/helper/test-oid-array.c b/t/helper/test-oid-array.c
index 0906993ad59..c3a1a00466a 100644
--- a/t/helper/test-oid-array.c
+++ b/t/helper/test-oid-array.c
@@ -9,7 +9,7 @@ static int print_oid(const struct object_id *oid, void *data)
 	return 0;
 }
 
-int cmd__oid_array(int argc, const char **argv)
+int cmd__oid_array(int argc UNUSED, const char **argv UNUSED)
 {
 	struct oid_array array = OID_ARRAY_INIT;
 	struct strbuf line = STRBUF_INIT;
diff --git a/t/helper/test-oidmap.c b/t/helper/test-oidmap.c
index 883d40efd45..ee92ae84949 100644
--- a/t/helper/test-oidmap.c
+++ b/t/helper/test-oidmap.c
@@ -22,7 +22,7 @@ struct test_entry {
  * iterate -> oidkey1 namevalue1\noidkey2 namevalue2\n...
  *
  */
-int cmd__oidmap(int argc, const char **argv)
+int cmd__oidmap(int argc UNUSED, const char **argv UNUSED)
 {
 	struct strbuf line = STRBUF_INIT;
 	struct oidmap map = OIDMAP_INIT;
diff --git a/t/helper/test-oidtree.c b/t/helper/test-oidtree.c
index 0b82431a70f..6a18e1bce80 100644
--- a/t/helper/test-oidtree.c
+++ b/t/helper/test-oidtree.c
@@ -3,13 +3,13 @@
 #include "hex.h"
 #include "oidtree.h"
 
-static enum cb_next print_oid(const struct object_id *oid, void *data)
+static enum cb_next print_oid(const struct object_id *oid, void *data UNUSED)
 {
 	puts(oid_to_hex(oid));
 	return CB_CONTINUE;
 }
 
-int cmd__oidtree(int argc, const char **argv)
+int cmd__oidtree(int argc UNUSED, const char **argv UNUSED)
 {
 	struct oidtree ot;
 	struct strbuf line = STRBUF_INIT;
diff --git a/t/helper/test-online-cpus.c b/t/helper/test-online-cpus.c
index 8cb0d53840f..47dc2117112 100644
--- a/t/helper/test-online-cpus.c
+++ b/t/helper/test-online-cpus.c
@@ -2,7 +2,7 @@
 #include "git-compat-util.h"
 #include "thread-utils.h"
 
-int cmd__online_cpus(int argc, const char **argv)
+int cmd__online_cpus(int argc UNUSED, const char **argv UNUSED)
 {
 	printf("%d\n", online_cpus());
 	return 0;
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 506835521a4..b66039e5751 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -263,14 +263,14 @@ int cmd__parse_options_flags(int argc, const char **argv)
 	return parse_options_flags__cmd(argc, argv, test_flags);
 }
 
-static int subcmd_one(int argc, const char **argv, const char *prefix)
+static int subcmd_one(int argc, const char **argv, const char *prefix UNUSED)
 {
 	printf("fn: subcmd_one\n");
 	print_args(argc, argv);
 	return 0;
 }
 
-static int subcmd_two(int argc, const char **argv, const char *prefix)
+static int subcmd_two(int argc, const char **argv, const char *prefix UNUSED)
 {
 	printf("fn: subcmd_two\n");
 	print_args(argc, argv);
diff --git a/t/helper/test-prio-queue.c b/t/helper/test-prio-queue.c
index 4915412e074..f0bf255f5f0 100644
--- a/t/helper/test-prio-queue.c
+++ b/t/helper/test-prio-queue.c
@@ -16,7 +16,7 @@ static void show(int *v)
 	free(v);
 }
 
-int cmd__prio_queue(int argc, const char **argv)
+int cmd__prio_queue(int argc UNUSED, const char **argv)
 {
 	struct prio_queue pq = { intcmp };
 
diff --git a/t/helper/test-read-graph.c b/t/helper/test-read-graph.c
index 98b73bb8f25..f92aea9d1fd 100644
--- a/t/helper/test-read-graph.c
+++ b/t/helper/test-read-graph.c
@@ -5,7 +5,7 @@
 #include "object-store.h"
 #include "bloom.h"
 
-int cmd__read_graph(int argc, const char **argv)
+int cmd__read_graph(int argc UNUSED, const char **argv UNUSED)
 {
 	struct commit_graph *graph = NULL;
 	struct object_directory *odb;
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 1745b088b7c..31c79a777a0 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -201,7 +201,8 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)
 	return ret;
 }
 
-static int cmd_for_each_reflog(struct ref_store *refs, const char **argv)
+static int cmd_for_each_reflog(struct ref_store *refs,
+			       const char **argv UNUSED)
 {
 	return refs_for_each_reflog(refs, each_ref, NULL);
 }
@@ -323,7 +324,7 @@ static struct command commands[] = {
 	{ NULL, NULL }
 };
 
-int cmd__ref_store(int argc, const char **argv)
+int cmd__ref_store(int argc UNUSED, const char **argv)
 {
 	struct ref_store *refs;
 	const char *func;
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index a26107ed70a..8a42c475b3b 100644
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -5,7 +5,7 @@
 #include "tree.h"
 #include "cache-tree.h"
 
-int cmd__scrap_cache_tree(int ac, const char **av)
+int cmd__scrap_cache_tree(int ac UNUSED, const char **av UNUSED)
 {
 	struct lock_file index_lock = LOCK_INIT;
 
diff --git a/t/helper/test-sigchain.c b/t/helper/test-sigchain.c
index d1cf7377b7c..2d5ecf73831 100644
--- a/t/helper/test-sigchain.c
+++ b/t/helper/test-sigchain.c
@@ -13,7 +13,7 @@ X(two)
 X(three)
 #undef X
 
-int cmd__sigchain(int argc, const char **argv)
+int cmd__sigchain(int argc UNUSED, const char **argv UNUSED)
 {
 	sigchain_push(SIGTERM, one);
 	sigchain_push(SIGTERM, two);
diff --git a/t/helper/test-strcmp-offset.c b/t/helper/test-strcmp-offset.c
index 44e4a6d143e..96b9a5b5291 100644
--- a/t/helper/test-strcmp-offset.c
+++ b/t/helper/test-strcmp-offset.c
@@ -1,7 +1,7 @@
 #include "test-tool.h"
 #include "cache.h"
 
-int cmd__strcmp_offset(int argc, const char **argv)
+int cmd__strcmp_offset(int argc UNUSED, const char **argv)
 {
 	int result;
 	size_t offset;
diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c
index 22a41c40926..edeee41abdd 100644
--- a/t/helper/test-submodule-config.c
+++ b/t/helper/test-submodule-config.c
@@ -4,7 +4,7 @@
 #include "submodule-config.h"
 #include "submodule.h"
 
-static void die_usage(int argc, const char **argv, const char *msg)
+static void die_usage(int argc UNUSED, const char **argv, const char *msg)
 {
 	fprintf(stderr, "%s\n", msg);
 	fprintf(stderr, "Usage: %s [<commit> <submodulepath>] ...\n", argv[0]);
diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c
index e060cc62268..3b75358723c 100644
--- a/t/helper/test-submodule.c
+++ b/t/helper/test-submodule.c
@@ -174,7 +174,7 @@ static int cmd__submodule_config_unset(int argc, const char **argv)
 	usage_with_options(usage, options);
 }
 
-static int cmd__submodule_config_writeable(int argc, const char **argv)
+static int cmd__submodule_config_writeable(int argc, const char **argv UNUSED)
 {
 	struct option options[] = {
 		OPT_END()
diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c
index f374c21ec32..688c8d017db 100644
--- a/t/helper/test-trace2.c
+++ b/t/helper/test-trace2.c
@@ -208,23 +208,23 @@ static int ut_007BUG(int argc, const char **argv)
 	BUG("the bug message");
 }
 
-static int ut_008bug(int argc, const char **argv)
+static int ut_008bug(int argc UNUSED, const char **argv UNUSED)
 {
 	bug("a bug message");
 	bug("another bug message");
 	BUG_if_bug("an explicit BUG_if_bug() following bug() call(s) is nice, but not required");
 	return 0;
 }
 
-static int ut_009bug_BUG(int argc, const char **argv)
+static int ut_009bug_BUG(int argc UNUSED, const char **argv UNUSED)
 {
 	bug("a bug message");
 	bug("another bug message");
 	/* The BUG_if_bug(...) isn't here, but we'll spot bug() calls on exit()! */
 	return 0;
 }
 
-static int ut_010bug_BUG(int argc, const char **argv)
+static int ut_010bug_BUG(int argc UNUSED, const char **argv UNUSED)
 {
 	bug("a %s message", "bug");
 	BUG("a %s message", "BUG");
diff --git a/t/helper/test-xml-encode.c b/t/helper/test-xml-encode.c
index a648bbd961c..b2f330d1a44 100644
--- a/t/helper/test-xml-encode.c
+++ b/t/helper/test-xml-encode.c
@@ -6,7 +6,7 @@ static const char *utf8_replace_character = "&#xfffd;";
  * Encodes (possibly incorrect) UTF-8 on <stdin> to <stdout>, to be embedded
  * in an XML file.
  */
-int cmd__xml_encode(int argc, const char **argv)
+int cmd__xml_encode(int argc UNUSED, const char **argv UNUSED)
 {
 	unsigned char buf[1024], tmp[4], *tmp2 = NULL;
 	ssize_t cur = 0, len = 1, remaining = 0;
-- 
2.40.0.675.gb17cd5d94c8

[PATCH 7/7] parse-options: drop parse_opt_unknown_cb()

[Jeff King]

This low-level callback was introduced in ce564eb1bd5 (parse-options:
add parse_opt_unknown_cb(), 2016-09-05) so that we could advertise
--indent-heuristic in git-blame's "-h" output, even though the option is
actually handled in parse_revision_opt(). We later stopped doing so in
44ae131e384 (builtin/blame.c: remove '--indent-heuristic' from usage
string, 2019-10-28).

This is a weird thing to do, and in the intervening years, we've never
used it again. Let's drop the helper in the name of simplicity.

Signed-off-by: Jeff King <peff@peff.net>
---
I guess this one is an oddball in this series, is it isn't about argv
directly. But it is argv-adjacent, and it needs to happen at some point
to silence the unused parameter warning.

 parse-options-cb.c | 15 ---------------
 parse-options.h    |  3 ---
 2 files changed, 18 deletions(-)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index d346dbe2100..4729277aa69 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -213,21 +213,6 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-/**
- * Report that the option is unknown, so that other code can handle
- * it. This can be used as a callback together with
- * OPTION_LOWLEVEL_CALLBACK to allow an option to be documented in the
- * "-h" output even if it's not being handled directly by
- * parse_options().
- */
-enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
-					   const struct option *opt,
-					   const char *arg, int unset)
-{
-	BUG_ON_OPT_ARG(arg);
-	return PARSE_OPT_UNKNOWN;
-}
-
 /**
  * Recreates the command-line option in the strbuf.
  */
diff --git a/parse-options.h b/parse-options.h
index 50d852f2991..60b0a758d6b 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -348,9 +348,6 @@ int parse_opt_commit(const struct option *, const char *, int);
 int parse_opt_tertiary(const struct option *, const char *, int);
 int parse_opt_string_list(const struct option *, const char *, int);
 int parse_opt_noop_cb(const struct option *, const char *, int);
-enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
-					   const struct option *,
-					   const char *, int);
 int parse_opt_passthru(const struct option *, const char *, int);
 int parse_opt_passthru_argv(const struct option *, const char *, int);
 /* value is enum branch_track* */
-- 
2.40.0.675.gb17cd5d94c8

Re: [PATCH 0/7] unused argc/argv/prefix parameters

[Derrick Stolee]

On 3/28/2023 4:52 PM, Jeff King wrote:
> More from my -Wunused-parameter silencing. The first one actually fixes
> a bug. The second is a cleanup, and the rest are just UNUSED annotations
> (but grouped into similar cases).

This was easy to read and LGTM.

Thanks,
-Stolee

Re: [PATCH 0/7] unused argc/argv/prefix parameters

[Junio C Hamano]

Derrick Stolee <derrickstolee@github.com> writes:

> On 3/28/2023 4:52 PM, Jeff King wrote:
>> More from my -Wunused-parameter silencing. The first one actually fixes
>> a bug. The second is a cleanup, and the rest are just UNUSED annotations
>> (but grouped into similar cases).
>
> This was easy to read and LGTM.

Thanks, both, for writing and reviewing.  I agree that the series
was a clear and easy read.

Let's merge it down to 'next'.