From 9dc607f1c29de88bc5194df49cc7f834e528a3e9 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:54:13 -0400 Subject: [PATCH 1/7] fast-import: fix file access when run from subdir 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 Signed-off-by: Junio C Hamano --- 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 f7548ff4a3..19427e05f1 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 a98ef032d9..410a871c52 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 \ + Date: Tue, 28 Mar 2023 16:54:32 -0400 Subject: [PATCH 2/7] builtins: always pass prefix to parse_options() 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 Signed-off-by: Junio C Hamano --- 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 42c2457c70..967a4442de 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 62986a7b1b..287721fd37 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); From 79156913774ef7c5f72264067e0764a1447a1bb3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:55:17 -0400 Subject: [PATCH 3/7] builtins: annotate always-empty prefix parameters 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 Signed-off-by: Junio C Hamano --- 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 46cc789789..cb0db67681 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 fd0e5f8683..462eefe102 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 491af9202d..8f8f2ac3e6 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 73509f651b..91e93f0c77 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 ee338bf440..282782eccd 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 b2a3980b1d..9020fab9c5 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 945ee2b412..7f9320ac6d 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); From 5247b762d0c22875c4ac71183e89b50d9f9acc2d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:56:55 -0400 Subject: [PATCH 4/7] builtins: mark unused prefix parameters 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 Signed-off-by: Junio C Hamano --- 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 d7b304fa08..7010752987 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 d1a4306da3..0d75e474f0 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 cae804a190..3d4f2ae1d0 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 c875f5d37e..b747b4ed98 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 284eb48609..c2e519301e 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 ... -- HEAD ..."; -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 b9acbf5d34..0a50d0a0ae 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 82115c5808..104d10877b 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 6a12fed271..5f327435b5 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1466,7 +1466,7 @@ done: 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 d05d1a8462..65a053261a 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2766,7 +2766,7 @@ cleanup: 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 e9b105a539..374d980c91 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 1908dcfcff..3d8510ccf8 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 d9943be9af..acb988d2d5 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; From 6ba21fa65cbdf97aac337a2857f8ffb072dda29c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:57:04 -0400 Subject: [PATCH 5/7] mark "argv" as unused when we check argc 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 Signed-off-by: Junio C Hamano --- 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 26f1e532c6..bc9da7bcb6 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 8f8f2ac3e6..4324d39fb4 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 ca19b95ce4..c7b8c16c8e 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")); From 126e3b3d2aa06c58994ed09e12cb0a0008897039 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:57:25 -0400 Subject: [PATCH 6/7] t/helper: mark unused argv/argc arguments 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 Signed-off-by: Junio C Hamano --- 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 534ca66441..71a1a5c9b0 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 45951b1df8..cd6a6df702 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 e37396dd9c..73e551cfc2 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 92dfc1aa8c..ba1c5ce717 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 975f0ac890..3d2fb92ade 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 813d0a38fa..4fabbdea49 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 af953fabe8..bc1ef8d4dc 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 7c7fc8efc1..2cf302ffcb 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 54a4856c48..0a1e492e5b 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 811e89c1bc..05f55eca21 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 fcd10968cc..a06c45c1f8 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 04bc2563f3..184aa1a087 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 0906993ad5..c3a1a00466 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 883d40efd4..ee92ae8494 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 0b82431a70..6a18e1bce8 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 8cb0d53840..47dc211711 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 506835521a..b66039e575 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 4915412e07..f0bf255f5f 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 98b73bb8f2..f92aea9d1f 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 1745b088b7..31c79a777a 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 a26107ed70..8a42c475b3 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 d1cf7377b7..2d5ecf7383 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 44e4a6d143..96b9a5b529 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 22a41c4092..edeee41abd 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 [ ] ...\n", argv[0]); diff --git a/t/helper/test-submodule.c b/t/helper/test-submodule.c index e060cc6226..3b75358723 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 f374c21ec3..688c8d017d 100644 --- a/t/helper/test-trace2.c +++ b/t/helper/test-trace2.c @@ -208,7 +208,7 @@ 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"); @@ -216,7 +216,7 @@ static int ut_008bug(int argc, const char **argv) 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"); @@ -224,7 +224,7 @@ static int ut_009bug_BUG(int argc, const char **argv) 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 a648bbd961..b2f330d1a4 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 = "�"; * Encodes (possibly incorrect) UTF-8 on to , 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; From 4a4d9706ade6dcdca7cc16372f1f83f5011a5d2b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 28 Mar 2023 16:58:42 -0400 Subject: [PATCH 7/7] parse-options: drop parse_opt_unknown_cb() 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 Signed-off-by: Junio C Hamano --- 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 d346dbe210..4729277aa6 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 50d852f299..60b0a758d6 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* */