{"thread":{"id":"58359","subject":"[PATCH 0/3] unused function parameters newly in next","startedAt":"2022-08-25T10:46:07Z","lastAt":"2022-08-25T16:49:41Z","messageCount":5,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"461942","messageId":"YwdSzrF3xaHvLsI+@coredump.intra.peff.net","threadId":"58359","inReplyTo":null,"subject":"[PATCH 0/3] unused function parameters newly in next","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-25T10:45:34Z","receivedAt":"2022-08-25T10:46:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The sg/parse-options-subcommand topic was merged to 'next', and it\nintroduced a ton of -Wunused-parameter warnings. But I think in this\ncase it revealed some actual bugs, albeit minor.\n\nHere are some fixes. I prepared them on top of that topic, which is\nsemantically necessary. But note that the topic is not itself at fault\nfor _introducing_ any bugs. It just revealed them. So I don't think\nthere's any need to pause its progress to master.\n\n  [1/3]: pass subcommand \"prefix\" arguments to parse_options()\n  [2/3]: maintenance: add parse-options boilerplate for subcommands\n  [3/3]: remote: run \"remote rm\" argv through parse_options()\n\n builtin/commit-graph.c     |  4 ++--\n builtin/gc.c               | 43 +++++++++++++++++++++++++++++++++++++-\n builtin/multi-pack-index.c |  8 +++----\n builtin/remote.c           | 36 ++++++++++++++++++-------------\n builtin/sparse-checkout.c  |  8 +++----\n 5 files changed, 73 insertions(+), 26 deletions(-)\n\n-Peff\n"},{"id":"461943","messageId":"YwdTJOVo46PrIHMc@coredump.intra.peff.net","threadId":"58359","inReplyTo":"YwdSzrF3xaHvLsI+@coredump.intra.peff.net","subject":"[PATCH 1/3] pass subcommand \"prefix\" arguments to parse_options()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-25T10:47:00Z","receivedAt":"2022-08-25T10:47:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Recent commits such as bf0a6b65fc (builtin/multi-pack-index.c: let\nparse-options parse subcommands, 2022-08-19) converted a few functions\nto match our usual argc/argv/prefix conventions, but the prefix argument\nremains unused.\n\nHowever, there is a good use for it: they should pass it to their own\nparse_options() functions, where it may be used to adjust the value of\nany filename options. In all but one of these functions, there's no\nbehavior change, since they don't use OPT_FILENAME. But this is an\nactual fix for one option, which you can see by modifying the test suite\nlike so:\n\n\tdiff --git a/t/t5326-multi-pack-bitmaps.sh b/t/t5326-multi-pack-bitmaps.sh\n\tindex 4fe57414c1..d0974d4371 100755\n\t--- a/t/t5326-multi-pack-bitmaps.sh\n\t+++ b/t/t5326-multi-pack-bitmaps.sh\n\t@@ -186,7 +186,11 @@ test_expect_success 'writing a bitmap with --refs-snapshot' '\n\n\t \t\t# Then again, but with a refs snapshot which only sees\n\t \t\t# refs/tags/one.\n\t-\t\tgit multi-pack-index write --bitmap --refs-snapshot=snapshot &&\n\t+\t\t(\n\t+\t\t\tmkdir subdir &&\n\t+\t\t\tcd subdir &&\n\t+\t\t\tgit multi-pack-index write --bitmap --refs-snapshot=../snapshot\n\t+\t\t) &&\n\n\t \t\ttest_path_is_file $midx &&\n\t \t\ttest_path_is_file $midx-$(midx_checksum $objdir).bitmap &&\n\nI'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken\nall along, because the sub-function never got to see the prefix. It is\nthat commit which is actually enabling us to fix it (and which also\nbrought attention to the problem because it triggers -Wunused-parameter!)\n\nThe other functions changed here don't use OPT_FILENAME at all. In their\ncases this isn't fixing anything visible, but it's following the usual\npattern and future-proofing them against somebody adding new options and\nbeing surprised.\n\nI didn't include a test for the one visible case above. We don't\ngenerally test routine parse-options behavior for individual options.\nThe challenge here was finding the problem, and now that this has been\ndone, it's not likely to regress. Likewise, we could apply the patch\nabove to cover it \"for free\" but it makes reading the rest of the test\nunnecessarily complicated.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/commit-graph.c     |  4 ++--\n builtin/multi-pack-index.c |  8 ++++----\n builtin/remote.c           | 28 ++++++++++++++++------------\n builtin/sparse-checkout.c  |  8 ++++----\n 4 files changed, 26 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 1eb5492cbd..dc3cc35394 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -80,7 +80,7 @@ static int graph_verify(int argc, const char **argv, const char *prefix)\n \ttrace2_cmd_mode(\"verify\");\n \n \topts.progress = isatty(2);\n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options,\n \t\t\t     builtin_commit_graph_verify_usage, 0);\n \tif (argc)\n@@ -241,7 +241,7 @@ static int graph_write(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_commit_graph_write_config, &opts);\n \n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options,\n \t\t\t     builtin_commit_graph_write_usage, 0);\n \tif (argc)\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex b8320d597b..248929f2e6 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -133,7 +133,7 @@ static int cmd_multi_pack_index_write(int argc, const char **argv,\n \n \tif (isatty(2))\n \t\topts.flags |= MIDX_PROGRESS;\n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options, builtin_multi_pack_index_write_usage,\n \t\t\t     0);\n \tif (argc)\n@@ -176,7 +176,7 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv,\n \n \tif (isatty(2))\n \t\topts.flags |= MIDX_PROGRESS;\n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options, builtin_multi_pack_index_verify_usage,\n \t\t\t     0);\n \tif (argc)\n@@ -203,7 +203,7 @@ static int cmd_multi_pack_index_expire(int argc, const char **argv,\n \n \tif (isatty(2))\n \t\topts.flags |= MIDX_PROGRESS;\n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options, builtin_multi_pack_index_expire_usage,\n \t\t\t     0);\n \tif (argc)\n@@ -233,7 +233,7 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv,\n \n \tif (isatty(2))\n \t\topts.flags |= MIDX_PROGRESS;\n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     options,\n \t\t\t     builtin_multi_pack_index_repack_usage,\n \t\t\t     0);\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 4a6d47c03a..96f562f00a 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -177,8 +177,8 @@ static int add(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,\n-\t\t\t     0);\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_add_usage, 0);\n \n \tif (argc != 2)\n \t\tusage_with_options(builtin_remote_add_usage, options);\n@@ -695,7 +695,7 @@ static int mv(int argc, const char **argv, const char *prefix)\n \tint i, refs_renamed_nr = 0, refspec_updated = 0;\n \tstruct progress *progress = NULL;\n \n-\targc = parse_options(argc, argv, NULL, options,\n+\targc = parse_options(argc, argv, prefix, options,\n \t\t\t     builtin_remote_rename_usage, 0);\n \n \tif (argc != 2)\n@@ -1264,7 +1264,8 @@ static int show(int argc, const char **argv, const char *prefix)\n \t};\n \tstruct show_info info = SHOW_INFO_INIT;\n \n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_show_usage,\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_show_usage,\n \t\t\t     0);\n \n \tif (argc < 1)\n@@ -1371,8 +1372,8 @@ static int set_head(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"delete refs/remotes/<name>/HEAD\")),\n \t\tOPT_END()\n \t};\n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_sethead_usage,\n-\t\t\t     0);\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_sethead_usage, 0);\n \tif (argc)\n \t\tstrbuf_addf(&buf, \"refs/remotes/%s/HEAD\", argv[0]);\n \n@@ -1471,8 +1472,8 @@ static int prune(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_prune_usage,\n-\t\t\t     0);\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_prune_usage, 0);\n \n \tif (argc < 1)\n \t\tusage_with_options(builtin_remote_prune_usage, options);\n@@ -1504,7 +1505,8 @@ static int update(int argc, const char **argv, const char *prefix)\n \tint default_defined = 0;\n \tint retval;\n \n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_update_usage,\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_update_usage,\n \t\t\t     PARSE_OPT_KEEP_ARGV0);\n \n \tstrvec_push(&fetch_argv, \"fetch\");\n@@ -1583,7 +1585,7 @@ static int set_branches(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, NULL, options,\n+\targc = parse_options(argc, argv, prefix, options,\n \t\t\t     builtin_remote_setbranches_usage, 0);\n \tif (argc == 0) {\n \t\terror(_(\"no remote specified\"));\n@@ -1608,7 +1610,8 @@ static int get_url(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"return all URLs\")),\n \t\tOPT_END()\n \t};\n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_geturl_usage, 0);\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_geturl_usage, 0);\n \n \tif (argc != 1)\n \t\tusage_with_options(builtin_remote_geturl_usage, options);\n@@ -1668,7 +1671,8 @@ static int set_url(int argc, const char **argv, const char *prefix)\n \t\t\t    N_(\"delete URLs\")),\n \t\tOPT_END()\n \t};\n-\targc = parse_options(argc, argv, NULL, options, builtin_remote_seturl_usage,\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_seturl_usage,\n \t\t\t     PARSE_OPT_KEEP_ARGV0);\n \n \tif (add_mode && delete_mode)\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 7b39a150a9..287716db68 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -60,7 +60,7 @@ static int sparse_checkout_list(int argc, const char **argv, const char *prefix)\n \tif (!core_apply_sparse_checkout)\n \t\tdie(_(\"this worktree is not sparse\"));\n \n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     builtin_sparse_checkout_list_options,\n \t\t\t     builtin_sparse_checkout_list_usage, 0);\n \n@@ -452,7 +452,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)\n \tinit_opts.cone_mode = -1;\n \tinit_opts.sparse_index = -1;\n \n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     builtin_sparse_checkout_init_options,\n \t\t\t     builtin_sparse_checkout_init_usage, 0);\n \n@@ -860,7 +860,7 @@ static int sparse_checkout_reapply(int argc, const char **argv,\n \treapply_opts.cone_mode = -1;\n \treapply_opts.sparse_index = -1;\n \n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     builtin_sparse_checkout_reapply_options,\n \t\t\t     builtin_sparse_checkout_reapply_usage, 0);\n \n@@ -897,7 +897,7 @@ static int sparse_checkout_disable(int argc, const char **argv,\n \t * forcibly return to a dense checkout regardless of initial state.\n \t */\n \n-\targc = parse_options(argc, argv, NULL,\n+\targc = parse_options(argc, argv, prefix,\n \t\t\t     builtin_sparse_checkout_disable_options,\n \t\t\t     builtin_sparse_checkout_disable_usage, 0);\n \n-- \n2.37.2.1034.gd926c9c740\n\n"},{"id":"461944","messageId":"YwdUGlRKkmZNM+OW@coredump.intra.peff.net","threadId":"58359","inReplyTo":"YwdSzrF3xaHvLsI+@coredump.intra.peff.net","subject":"[PATCH 2/3] maintenance: add parse-options boilerplate for subcommands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-25T10:51:06Z","receivedAt":"2022-08-25T10:51:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Several of the git-maintenance subcommands don't take any options, so\nthey don't bother looking at argv at all. This means they'll silently\naccept garbage, like:\n\n  $ git maintenance register --foo\n  [no output]\n\n  $ git maintenance stop bar\n  [no output]\n\nLet's give them the basic boilerplate to detect and handle these cases:\n\n  $ git maintenance register --foo\n  error: unknown option `foo'\n  usage: git maintenance register\n\n  $ git maintenance stop bar\n  usage: git maintenance stop\n\nWe could reduce the number of lines of code here a bit with a shared\nhelper function. But it's worth building out the boilerplate, as it may\nserve as the base for adding options later.\n\nNote one complication: maintenance_start() calls directly into\nmaintenance_register(), so it now needs to pass a plausible argv (we\ndon't care, but parse_options() is expecting there to at least be an\nargv[0] program name). This is an extra line of code, but it eliminates\nthe need for an explanatory comment.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI kind of hate the register_args thing from the last paragraph.\nparse-options is actually capable of handling a 0-length argc except\nthat it blindly walks past the first argument at the start. So loosening\nit like this:\n\n  diff --git a/parse-options.c b/parse-options.c\n  index a1ec932f0f..c95ecd366a 100644\n  --- a/parse-options.c\n  +++ b/parse-options.c\n  @@ -538,7 +538,7 @@ static void parse_options_start_1(struct parse_opt_ctx_t *ctx,\n   {\n          ctx->argc = argc;\n          ctx->argv = argv;\n  -       if (!(flags & PARSE_OPT_ONE_SHOT)) {\n  +       if (!(flags & PARSE_OPT_ONE_SHOT) && ctx->argc) {\n                  ctx->argc--;\n                  ctx->argv++;\n          }\n\nmakes the original code just work. But I dunno. It feels kind of subtle.\nThe solution in the patch below is ugly but fairly straightforward.\n\n builtin/gc.c | 43 ++++++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 42 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 19d6b3b558..84549888f5 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1465,14 +1465,28 @@ static char *get_maintpath(void)\n \treturn strbuf_detach(&sb, NULL);\n }\n \n+static char const * const builtin_maintenance_register_usage[] = {\n+\tN_(\"git maintenance register\"),\n+\tNULL\n+};\n+\n static int maintenance_register(int argc, const char **argv, const char *prefix)\n {\n+\tstruct option options[] = {\n+\t\tOPT_END(),\n+\t};\n \tint rc;\n \tchar *config_value;\n \tstruct child_process config_set = CHILD_PROCESS_INIT;\n \tstruct child_process config_get = CHILD_PROCESS_INIT;\n \tchar *maintpath = get_maintpath();\n \n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_maintenance_register_usage, 0);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_register_usage,\n+\t\t\t\t   options);\n+\n \t/* Disable foreground maintenance */\n \tgit_config_set(\"maintenance.auto\", \"false\");\n \n@@ -1509,12 +1523,26 @@ static int maintenance_register(int argc, const char **argv, const char *prefix)\n \treturn rc;\n }\n \n+static char const * const builtin_maintenance_unregister_usage[] = {\n+\tN_(\"git maintenance unregister\"),\n+\tNULL\n+};\n+\n static int maintenance_unregister(int argc, const char **argv, const char *prefix)\n {\n+\tstruct option options[] = {\n+\t\tOPT_END(),\n+\t};\n \tint rc;\n \tstruct child_process config_unset = CHILD_PROCESS_INIT;\n \tchar *maintpath = get_maintpath();\n \n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_maintenance_unregister_usage, 0);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_unregister_usage,\n+\t\t\t\t   options);\n+\n \tconfig_unset.git_cmd = 1;\n \tstrvec_pushl(&config_unset.args, \"config\", \"--global\", \"--unset\",\n \t\t     \"--fixed-value\", \"maintenance.repo\", maintpath, NULL);\n@@ -2496,6 +2524,7 @@ static int maintenance_start(int argc, const char **argv, const char *prefix)\n \t\t\tPARSE_OPT_NONEG, maintenance_opt_scheduler),\n \t\tOPT_END()\n \t};\n+\tconst char *register_args[] = { \"register\", NULL };\n \n \targc = parse_options(argc, argv, prefix, options,\n \t\t\t     builtin_maintenance_start_usage, 0);\n@@ -2505,13 +2534,25 @@ static int maintenance_start(int argc, const char **argv, const char *prefix)\n \topts.scheduler = resolve_scheduler(opts.scheduler);\n \tvalidate_scheduler(opts.scheduler);\n \n-\tif (maintenance_register(0, NULL, NULL)) /* It doesn't take any args */\n+\tif (maintenance_register(ARRAY_SIZE(register_args)-1, register_args, NULL))\n \t\twarning(_(\"failed to add repo to global config\"));\n \treturn update_background_schedule(&opts, 1);\n }\n \n+static const char *const builtin_maintenance_stop_usage[] = {\n+\tN_(\"git maintenance stop\"),\n+\tNULL\n+};\n+\n static int maintenance_stop(int argc, const char **argv, const char *prefix)\n {\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_maintenance_stop_usage, 0);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_stop_usage, options);\n \treturn update_background_schedule(NULL, 0);\n }\n \n-- \n2.37.2.1034.gd926c9c740\n\n"},{"id":"461945","messageId":"YwdUPIv097AWRfTI@coredump.intra.peff.net","threadId":"58359","inReplyTo":"YwdSzrF3xaHvLsI+@coredump.intra.peff.net","subject":"[PATCH 3/3] remote: run \"remote rm\" argv through parse_options()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-25T10:51:40Z","receivedAt":"2022-08-25T10:51:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"git remote rm\" command's option parsing is fairly primitive: it\ninsists on a single argument, which it treats as the remote name, and\ndisplays a usage message otherwise.\n\nThis is OK, and maybe even convenient, as you could run:\n\n  git remote rm --foo\n\nto drop a remote named \"--foo\". But it's also weirdly unlike most of the\nrest of Git, which would complain that there is no option \"--foo\". The\nright way to spell it by our conventions is:\n\n  git remote rm -- --foo\n\nbut this doesn't currently work.\n\nSo let's bring the command in line with the rest of Git (including its\nsibling subcommands!) by feeding argv to parse_options(). We already\nhave an empty options array for the usage helper.\n\nNote that we have to adjust the argc index down by one, as\nparse_options() eats the program name from the start of the array.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/remote.c | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 96f562f00a..9aff864fd6 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -862,12 +862,14 @@ static int rm(int argc, const char **argv, const char *prefix)\n \tcb_data.skipped = &skipped;\n \tcb_data.keep = &known_remotes;\n \n-\tif (argc != 2)\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t     builtin_remote_rm_usage, 0);\n+\tif (argc != 1)\n \t\tusage_with_options(builtin_remote_rm_usage, options);\n \n-\tremote = remote_get(argv[1]);\n+\tremote = remote_get(argv[0]);\n \tif (!remote_is_configured(remote, 1)) {\n-\t\terror(_(\"No such remote: '%s'\"), argv[1]);\n+\t\terror(_(\"No such remote: '%s'\"), argv[0]);\n \t\texit(2);\n \t}\n \n-- \n2.37.2.1034.gd926c9c740\n"},{"id":"461963","messageId":"xmqq8rnc2rue.fsf@gitster.g","threadId":"58359","inReplyTo":"YwdTJOVo46PrIHMc@coredump.intra.peff.net","subject":"Re: [PATCH 1/3] pass subcommand \"prefix\" arguments to parse_options()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-25T16:47:05Z","receivedAt":"2022-08-25T16:49:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> However, there is a good use for it: they should pass it to their own\n> parse_options() functions, where it may be used to adjust the value of\n> any filename options.\n\nOh, absolutely.  Not passing prefix if they use OPT_FILENAME would\nbe a bug.\n\n> I'd emphasize that this wasn't broken by bf0a6b65fc; it has been broken\n> all along, because the sub-function never got to see the prefix. It is\n> that commit which is actually enabling us to fix it (and which also\n> brought attention to the problem because it triggers -Wunused-parameter!)\n\nYeah.  I like that analysis.  With the commit, we pass prefix\nthrough the callgraph, and now we can fix it ;-)\n\nThanks.\n"}]}