{"thread":{"id":"54328","subject":"[PATCH 0/10] dropping more unused parameters","startedAt":"2020-09-30T12:27:35Z","lastAt":"2020-09-30T12:35:03Z","messageCount":11,"participants":["Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":10},"messages":[{"id":"406666","messageId":"20200930122732.GA1901036@coredump.intra.peff.net","threadId":"54328","inReplyTo":null,"subject":"[PATCH 0/10] dropping more unused parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:27:32Z","receivedAt":"2020-09-30T12:27:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is the last of my series of patches to drop (or use) unused\nfunction parameters. I've been holding on to some of these for almost 2\nyears, because I wanted to make sure they weren't actually bugs\n(dropping unused parameters can never _introduce_ a bug, but it's a good\nopportunity to investigate whether the parameter _should_ have been\nused). I'm still not entirely convinced that patch 10 isn't actually a\nbug, but I wasn't able to puzzle it out either way, and it didn't seem\nworth holding up the topic any longer.\n\nAfter this lands, I have a follow-on series that annotates cases where\nwe can't drop parameters (e.g., functions which have to conform to a\ncallback interface, but don't care about some of the parameters). And\nthen after that we can flip on -Wunused-parameters going forward. Which\nI think is worth doing, as it has found some bugs (fixed in earlier\nseries).\n\n [01/10]: convert: drop unused crlf_action from check_global_conv_flags_eol()\n [02/10]: drop unused argc parameters\n [03/10]: env--helper: write to opt->value in parseopt helper\n [04/10]: assert PARSE_OPT_NONEG in parse-options callbacks\n [05/10]: push: drop unused repo argument to do_push()\n [06/10]: sequencer: drop repository argument from run_git_commit()\n [07/10]: sparse-checkout: fill in some options boilerplate\n [08/10]: test-advise: check argument count with argc instead of argv\n [09/10]: sequencer: handle ignore_footer when parsing trailers\n [10/10]: dir.c: drop unused \"untracked\" from treat_path_fast()\n\n builtin/add.c | 4 +--\n builtin/am.c | 2 ++\n builtin/commit-graph.c | 2 ++\n builtin/commit.c | 12 +++----\n builtin/env--helper.c | 13 ++++---\n builtin/push.c | 4 +--\n builtin/sparse-checkout.c | 37 ++++++++++++++++++++\n commit.h | 2 +-\n convert.c | 4 +--\n dir.c | 3 +-\n parse-options-cb.c | 2 ++\n revision.c | 6 ++--\n sequencer.c | 20 +++++++----\n t/helper/test-advise.c | 4 +--\n t/helper/test-submodule-nested-repo-config.c | 6 ++--\n 15 files changed, 88 insertions(+), 33 deletions(-)\n\n-Peff\n"},{"id":"406667","messageId":"20200930122753.GA1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 01/10] convert: drop unused crlf_action from check_global_conv_flags_eol()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:27:53Z","receivedAt":"2020-09-30T12:27:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The crlf_action parameter hasn't been used since a0ad53c181 (convert:\nCorrect NNO tests and missing `LF will be replaced by CRLF`,\n2016-08-13), where that part of the function was hoisted out to a\nseparate will_convert_lf_to_crlf() helper. Let's drop the useless\nparameter.\n\nSigned-off-by: Jeff King \n---\n convert.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/convert.c b/convert.c\nindex 8e6c292421..ee360c2f07 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -195,7 +195,7 @@ static enum eol output_eol(enum crlf_action crlf_action)\n \treturn core_eol;\n }\n \n-static void check_global_conv_flags_eol(const char *path, enum crlf_action crlf_action,\n+static void check_global_conv_flags_eol(const char *path,\n \t\t\t struct text_stat *old_stats, struct text_stat *new_stats,\n \t\t\t int conv_flags)\n {\n@@ -547,7 +547,7 @@ static int crlf_to_git(const struct index_state *istate,\n \t\t\tnew_stats.crlf += new_stats.lonelf;\n \t\t\tnew_stats.lonelf = 0;\n \t\t}\n-\t\tcheck_global_conv_flags_eol(path, crlf_action, &stats, &new_stats, conv_flags);\n+\t\tcheck_global_conv_flags_eol(path, &stats, &new_stats, conv_flags);\n \t}\n \tif (!convert_crlf_into_lf)\n \t\treturn 0;\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406668","messageId":"20200930122818.GB1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 02/10] drop unused argc parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:28:18Z","receivedAt":"2020-09-30T12:28:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Many functions take an argv/argc pair, but never actually look at argc.\nThis makes it useless at best (we use the NULL sentinel in argv to find\nthe end of the array), and misleading at worst (what happens if the argc\ncount does not match the argv NULL?).\n\nIn each of these instances, the argv NULL does match the argc count, so\nthere are no bugs here. But let's tighten the interfaces to make it\nharder to get wrong (and to reduce some -Wunused-parameter complaints).\n\nSigned-off-by: Jeff King \n---\n builtin/add.c | 4 ++--\n builtin/commit.c | 12 ++++++------\n commit.h | 2 +-\n revision.c | 6 +++---\n t/helper/test-submodule-nested-repo-config.c | 6 +++---\n 5 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 26b6ced09e..a825887c50 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -239,7 +239,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \treturn status;\n }\n \n-int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n+int interactive_add(const char **argv, const char *prefix, int patch)\n {\n \tstruct pathspec pathspec;\n \n@@ -451,7 +451,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (add_interactive) {\n \t\tif (pathspec_from_file)\n \t\t\tdie(_(\"--pathspec-from-file is incompatible with --interactive/--patch\"));\n-\t\texit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));\n+\t\texit(interactive_add(argv + 1, prefix, patch_interactive));\n \t}\n \tif (legacy_stash_p) {\n \t\tstruct pathspec pathspec;\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 42b964e0ca..1dfd799ec5 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -326,7 +326,7 @@ static void refresh_cache_or_die(int refresh_flags)\n \t\tdie_resolve_conflict(\"commit\");\n }\n \n-static const char *prepare_index(int argc, const char **argv, const char *prefix,\n+static const char *prepare_index(const char **argv, const char *prefix,\n \t\t\t\t const struct commit *current_head, int is_status)\n {\n \tstruct string_list partial = STRING_LIST_INIT_DUP;\n@@ -378,7 +378,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\told_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT));\n \t\tsetenv(INDEX_ENVIRONMENT, the_repository->index_file, 1);\n \n-\t\tif (interactive_add(argc, argv, prefix, patch_interactive) != 0)\n+\t\tif (interactive_add(argv, prefix, patch_interactive) != 0)\n \t\t\tdie(_(\"interactive add failed\"));\n \n \t\tthe_repository->index_file = old_repo_index_file;\n@@ -1241,13 +1241,13 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \treturn argc;\n }\n \n-static int dry_run_commit(int argc, const char **argv, const char *prefix,\n+static int dry_run_commit(const char **argv, const char *prefix,\n \t\t\t const struct commit *current_head, struct wt_status *s)\n {\n \tint committable;\n \tconst char *index_file;\n \n-\tindex_file = prepare_index(argc, argv, prefix, current_head, 1);\n+\tindex_file = prepare_index(argv, prefix, current_head, 1);\n \tcommittable = run_status(stdout, index_file, prefix, 0, s);\n \trollback_index_files();\n \n@@ -1584,8 +1584,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tverbose = (config_commit_verbose < 0) ? 0 : config_commit_verbose;\n \n \tif (dry_run)\n-\t\treturn dry_run_commit(argc, argv, prefix, current_head, &s);\n-\tindex_file = prepare_index(argc, argv, prefix, current_head, 0);\n+\t\treturn dry_run_commit(argv, prefix, current_head, &s);\n+\tindex_file = prepare_index(argv, prefix, current_head, 0);\n \n \t/* Set up everything for writing the commit object. This includes\n \t running hooks, writing the trees, and interacting with the user. */\ndiff --git a/commit.h b/commit.h\nindex e6f8f7c26f..5467786c7b 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -248,7 +248,7 @@ struct oid_array;\n struct ref;\n int for_each_commit_graft(each_commit_graft_fn, void *);\n \n-int interactive_add(int argc, const char **argv, const char *prefix, int patch);\n+int interactive_add(const char **argv, const char *prefix, int patch);\n int run_add_interactive(const char *revision, const char *patch_mode,\n \t\t\tconst struct pathspec *pathspec);\n \ndiff --git a/revision.c b/revision.c\nindex d9dc5781ac..aa62212040 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2580,8 +2580,8 @@ static int for_each_good_bisect_ref(struct ref_store *refs, each_ref_fn fn, void\n }\n \n static int handle_revision_pseudo_opt(const char *submodule,\n-\t\t\t\tstruct rev_info *revs,\n-\t\t\t\tint argc, const char **argv, int *flags)\n+\t\t\t\t struct rev_info *revs,\n+\t\t\t\t const char **argv, int *flags)\n {\n \tconst char *arg = argv[0];\n \tconst char *optarg;\n@@ -2752,7 +2752,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tint opts;\n \n \t\t\topts = handle_revision_pseudo_opt(submodule,\n-\t\t\t\t\t\trevs, argc - i, argv + i,\n+\t\t\t\t\t\trevs, argv + i,\n \t\t\t\t\t\t&flags);\n \t\t\tif (opts > 0) {\n \t\t\t\ti += opts - 1;\ndiff --git a/t/helper/test-submodule-nested-repo-config.c b/t/helper/test-submodule-nested-repo-config.c\nindex bc97929bbc..c5fd4527dc 100644\n--- a/t/helper/test-submodule-nested-repo-config.c\n+++ b/t/helper/test-submodule-nested-repo-config.c\n@@ -1,7 +1,7 @@\n #include \"test-tool.h\"\n #include \"submodule-config.h\"\n \n-static void die_usage(int argc, const char **argv, const char *msg)\n+static void die_usage(const char **argv, const char *msg)\n {\n \tfprintf(stderr, \"%s\\n\", msg);\n \tfprintf(stderr, \"Usage: %s \\n\", argv[0]);\n@@ -14,13 +14,13 @@ int cmd__submodule_nested_repo_config(int argc, const char **argv)\n \tconst struct submodule *sub;\n \n \tif (argc < 3)\n-\t\tdie_usage(argc, argv, \"Wrong number of arguments.\");\n+\t\tdie_usage(argv, \"Wrong number of arguments.\");\n \n \tsetup_git_directory();\n \n \tsub = submodule_from_path(the_repository, &null_oid, argv[1]);\n \tif (repo_submodule_init(&subrepo, the_repository, sub)) {\n-\t\tdie_usage(argc, argv, \"Submodule not found.\");\n+\t\tdie_usage(argv, \"Submodule not found.\");\n \t}\n \n \t/* Read the config of _child_ submodules. */\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406669","messageId":"20200930122832.GC1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 03/10] env--helper: write to opt->value in parseopt helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:28:32Z","receivedAt":"2020-09-30T12:28:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We use OPT_CALLBACK_F() to call the option_parse_type() callback,\npassing it the address of \"cmdmode\" as the value to write to. But the\ncallback doesn't look at opt->value at all, and instead writes to a\nglobal variable.\n\nThis works out because that's the same global variable we happen to pass\nin, but it's rather confusing. Let's use the passed-in value instead.\nWe'll also make \"cmdmode\" a local variable of the main function,\nensuring we can't make the same mistake again.\n\nSigned-off-by: Jeff King \n---\n builtin/env--helper.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/env--helper.c b/builtin/env--helper.c\nindex 23c214fff6..3aa4282114 100644\n--- a/builtin/env--helper.c\n+++ b/builtin/env--helper.c\n@@ -7,18 +7,20 @@ static char const * const env__helper_usage[] = {\n \tNULL\n };\n \n-static enum {\n+enum cmdmode {\n \tENV_HELPER_TYPE_BOOL = 1,\n \tENV_HELPER_TYPE_ULONG\n-} cmdmode = 0;\n+};\n \n static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\t int unset)\n {\n+\tenum cmdmode *cmdmode = opt->value;\n+\n \tif (!strcmp(arg, \"bool\"))\n-\t\tcmdmode = ENV_HELPER_TYPE_BOOL;\n+\t\t*cmdmode = ENV_HELPER_TYPE_BOOL;\n \telse if (!strcmp(arg, \"ulong\"))\n-\t\tcmdmode = ENV_HELPER_TYPE_ULONG;\n+\t\t*cmdmode = ENV_HELPER_TYPE_ULONG;\n \telse\n \t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n \n@@ -33,6 +35,7 @@ int cmd_env__helper(int argc, const char **argv, const char *prefix)\n \tint ret;\n \tint ret_int, default_int;\n \tunsigned long ret_ulong, default_ulong;\n+\tenum cmdmode cmdmode = 0;\n \tstruct option opts[] = {\n \t\tOPT_CALLBACK_F(0, \"type\", &cmdmode, N_(\"type\"),\n \t\t\t N_(\"value is given this type\"), PARSE_OPT_NONEG,\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406670","messageId":"20200930122902.GD1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 04/10] assert PARSE_OPT_NONEG in parse-options callbacks","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:29:02Z","receivedAt":"2020-09-30T12:29:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In the spirit of 517fe807d6 (assert NOARG/NONEG behavior of\nparse-options callbacks, 2018-11-05), let's cover some parse-options\ncallbacks which expect to be used with PARSE_OPT_NONEG but don't\nexplicitly assert that this is the case. These callbacks are all used\ncorrectly in the current code, but this will help document their\nexpectations and future-proof the code.\n\nAs a bonus, it also silences -Wunused-parameters (these were added since\nthe initial sweep of 517fe807d6, and we can't yet turn on\n-Wunused-parameters to remind people because it has too many existing\nfalse positives).\n\nSigned-off-by: Jeff King \n---\n builtin/am.c | 2 ++\n builtin/commit-graph.c | 2 ++\n builtin/env--helper.c | 2 ++\n parse-options-cb.c | 2 ++\n 4 files changed, 8 insertions(+)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 7259186408..2c7673f74e 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -2180,6 +2180,8 @@ static int parse_opt_show_current_patch(const struct option *opt, const char *ar\n \t};\n \tint new_value = SHOW_PATCH_RAW;\n \n+\tBUG_ON_OPT_NEG(unset);\n+\n \tif (arg) {\n \t\tfor (new_value = 0; new_value < ARRAY_SIZE(valid_modes); new_value++) {\n \t\t\tif (!strcmp(arg, valid_modes[new_value]))\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 988445abdf..78fa08f43a 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -128,6 +128,8 @@ static int write_option_parse_split(const struct option *opt, const char *arg,\n {\n \tenum commit_graph_split_flags *flags = opt->value;\n \n+\tBUG_ON_OPT_NEG(unset);\n+\n \topts.split = 1;\n \tif (!arg)\n \t\treturn 0;\ndiff --git a/builtin/env--helper.c b/builtin/env--helper.c\nindex 3aa4282114..27349098b0 100644\n--- a/builtin/env--helper.c\n+++ b/builtin/env--helper.c\n@@ -17,6 +17,8 @@ static int option_parse_type(const struct option *opt, const char *arg,\n {\n \tenum cmdmode *cmdmode = opt->value;\n \n+\tBUG_ON_OPT_NEG(unset);\n+\n \tif (!strcmp(arg, \"bool\"))\n \t\t*cmdmode = ENV_HELPER_TYPE_BOOL;\n \telse if (!strcmp(arg, \"ulong\"))\ndiff --git a/parse-options-cb.c b/parse-options-cb.c\nindex d9d3b0819f..4542d4d3f9 100644\n--- a/parse-options-cb.c\n+++ b/parse-options-cb.c\n@@ -105,6 +105,8 @@ int parse_opt_commit(const struct option *opt, const char *arg, int unset)\n \tstruct commit *commit;\n \tstruct commit **target = opt->value;\n \n+\tBUG_ON_OPT_NEG(unset);\n+\n \tif (!arg)\n \t\treturn -1;\n \tif (get_oid(arg, &oid))\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406671","messageId":"20200930122909.GE1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 05/10] push: drop unused repo argument to do_push()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:29:09Z","receivedAt":"2020-09-30T12:29:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We stopped using the \"repo\" argument in 8e4c8af058 (push: disallow --all\nand refspecs when remote..mirror is set, 2019-09-02), which moved\nthe pushremote handling to its caller.\n\nSigned-off-by: Jeff King \n---\n builtin/push.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 0eeb2c8dd5..6da3a8e5d3 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -379,7 +379,7 @@ static int push_with_options(struct transport *transport, struct refspec *rs,\n \treturn 1;\n }\n \n-static int do_push(const char *repo, int flags,\n+static int do_push(int flags,\n \t\t const struct string_list *push_options,\n \t\t struct remote *remote)\n {\n@@ -629,7 +629,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)\n \t\tif (strchr(item->string, '\\n'))\n \t\t\tdie(_(\"push options must not have new line characters\"));\n \n-\trc = do_push(repo, flags, push_options, remote);\n+\trc = do_push(flags, push_options, remote);\n \tstring_list_clear(&push_options_cmdline, 0);\n \tstring_list_clear(&push_options_config, 0);\n \tif (rc == -1)\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406672","messageId":"20200930122931.GF1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 06/10] sequencer: drop repository argument from run_git_commit()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:29:31Z","receivedAt":"2020-09-30T12:29:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When we switched to using an external git-commit call in b0a3186140\n(sequencer: simplify root commit creation, 2019-08-19), this function\ndidn't need to care about the repository object any more.\n\nArguably we could be passing along the repository path to the external\ngit-commit by using \"--git-dir=r->path\" here. But for the most part the\nsequencer code relies on sub-process finding the same repository we're\nalready in (using the same environment variables or discovery process we\ndid). But we don't have a convenient interface for doing so, and there's\nno indication that we need to. Let's just drop the unused parameter for\nnow.\n\nSigned-off-by: Jeff King \n---\n sequencer.c | 11 +++++------\n 1 file changed, 5 insertions(+), 6 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex e8676e965f..6e9aabaac1 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -934,8 +934,7 @@ static int run_command_silent_on_success(struct child_process *cmd)\n * interactive rebase: in that case, we will want to retain the\n * author metadata.\n */\n-static int run_git_commit(struct repository *r,\n-\t\t\t const char *defmsg,\n+static int run_git_commit(const char *defmsg,\n \t\t\t struct replay_opts *opts,\n \t\t\t unsigned int flags)\n {\n@@ -1545,7 +1544,7 @@ static int do_commit(struct repository *r,\n \t\tif (is_rebase_i(opts) && oid)\n \t\t\tif (write_rebase_head(oid))\n \t\t\t return -1;\n-\t\treturn run_git_commit(r, msg_file, opts, flags);\n+\t\treturn run_git_commit(msg_file, opts, flags);\n \t}\n \n \treturn res;\n@@ -2060,7 +2059,7 @@ static int do_pick_commit(struct repository *r,\n \t\t*check_todo = !!(flags & EDIT_MSG);\n \t\tif (!res && reword) {\n fast_forward_edit:\n-\t\t\tres = run_git_commit(r, NULL, opts, EDIT_MSG |\n+\t\t\tres = run_git_commit(NULL, opts, EDIT_MSG |\n \t\t\t\t\t VERIFY_MSG | AMEND_MSG |\n \t\t\t\t\t (flags & ALLOW_EMPTY));\n \t\t\t*check_todo = 1;\n@@ -3748,7 +3747,7 @@ static int do_merge(struct repository *r,\n \t\t * command needs to be rescheduled).\n \t\t */\n \tfast_forward_edit:\n-\t\tret = !!run_git_commit(r, git_path_merge_msg(r), opts,\n+\t\tret = !!run_git_commit(git_path_merge_msg(r), opts,\n \t\t\t\t run_commit_flags);\n \n leave_merge:\n@@ -4437,7 +4436,7 @@ static int commit_staged_changes(struct repository *r,\n \t\t\treturn 0;\n \t}\n \n-\tif (run_git_commit(r, final_fixup ? NULL : rebase_path_message(),\n+\tif (run_git_commit(final_fixup ? NULL : rebase_path_message(),\n \t\t\t opts, flags))\n \t\treturn error(_(\"could not commit staged changes.\"));\n \tunlink(rebase_path_amend());\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406673","messageId":"20200930123010.GG1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 07/10] sparse-checkout: fill in some options boilerplate","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:30:10Z","receivedAt":"2020-09-30T12:30:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The sparse-checkout passes along argv and argc to its sub-command helper\nfunctions. Many of these sub-commands do not yet take any command-line\noptions, and ignore those parameters.\n\nLet's instead add empty option lists and make sure we call\nparse_options(). That will give a useful error message for something\nlike:\n\n git sparse-checkout list --nonsense\n\nwhich currently just silently ignores the unknown option.\n\nAs a bonus, it also silences some -Wunused-parameter warnings.\n\nSigned-off-by: Jeff King \n---\n builtin/sparse-checkout.c | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n\ndiff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c\nindex 4003f4d13a..e3140db2a0 100644\n--- a/builtin/sparse-checkout.c\n+++ b/builtin/sparse-checkout.c\n@@ -46,12 +46,24 @@ static void write_patterns_to_file(FILE *fp, struct pattern_list *pl)\n \t}\n }\n \n+static char const * const builtin_sparse_checkout_list_usage[] = {\n+\tN_(\"git sparse-checkout list\"),\n+\tNULL\n+};\n+\n static int sparse_checkout_list(int argc, const char **argv)\n {\n+\tstatic struct option builtin_sparse_checkout_list_options[] = {\n+\t\tOPT_END(),\n+\t};\n \tstruct pattern_list pl;\n \tchar *sparse_filename;\n \tint res;\n \n+\targc = parse_options(argc, argv, NULL,\n+\t\t\t builtin_sparse_checkout_list_options,\n+\t\t\t builtin_sparse_checkout_list_usage, 0);\n+\n \tmemset(&pl, 0, sizeof(pl));\n \n \tpl.use_cone_patterns = core_sparse_checkout_cone;\n@@ -560,17 +572,42 @@ static int sparse_checkout_set(int argc, const char **argv, const char *prefix,\n \treturn modify_pattern_list(argc, argv, m);\n }\n \n+static char const * const builtin_sparse_checkout_reapply_usage[] = {\n+\tN_(\"git sparse-checkout reapply\"),\n+\tNULL\n+};\n+\n static int sparse_checkout_reapply(int argc, const char **argv)\n {\n+\tstatic struct option builtin_sparse_checkout_reapply_options[] = {\n+\t\tOPT_END(),\n+\t};\n+\n+\targc = parse_options(argc, argv, NULL,\n+\t\t\t builtin_sparse_checkout_reapply_options,\n+\t\t\t builtin_sparse_checkout_reapply_usage, 0);\n+\n \trepo_read_index(the_repository);\n \treturn update_working_directory(NULL);\n }\n \n+static char const * const builtin_sparse_checkout_disable_usage[] = {\n+\tN_(\"git sparse-checkout disable\"),\n+\tNULL\n+};\n+\n static int sparse_checkout_disable(int argc, const char **argv)\n {\n+\tstatic struct option builtin_sparse_checkout_disable_options[] = {\n+\t\tOPT_END(),\n+\t};\n \tstruct pattern_list pl;\n \tstruct strbuf match_all = STRBUF_INIT;\n \n+\targc = parse_options(argc, argv, NULL,\n+\t\t\t builtin_sparse_checkout_disable_options,\n+\t\t\t builtin_sparse_checkout_disable_usage, 0);\n+\n \trepo_read_index(the_repository);\n \n \tmemset(&pl, 0, sizeof(pl));\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406674","messageId":"20200930123027.GH1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 08/10] test-advise: check argument count with argc instead of argv","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:30:27Z","receivedAt":"2020-09-30T12:30:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We complain if \"test-tool advise\" is not given an argument, but we\nquietly ignore any additional arguments it receives. Let's instead check\nthat we got the expected number. As a bonus, this silences\n-Wunused-parameter, which notes that we don't ever look at argc.\n\nWhile we're here, we can also fix the indentation in the conditional.\n\nSigned-off-by: Jeff King \n---\n t/helper/test-advise.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-advise.c b/t/helper/test-advise.c\nindex 38cdc2884e..a7043df1d3 100644\n--- a/t/helper/test-advise.c\n+++ b/t/helper/test-advise.c\n@@ -5,8 +5,8 @@\n \n int cmd__advise_if_enabled(int argc, const char **argv)\n {\n-\tif (!argv[1])\n-\tdie(\"usage: %s \", argv[0]);\n+\tif (argc != 2)\n+\t\tdie(\"usage: %s \", argv[0]);\n \n \tsetup_git_directory();\n \tgit_config(git_default_config, NULL);\n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406675","messageId":"20200930123411.GI1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 09/10] sequencer: handle ignore_footer when parsing trailers","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:34:11Z","receivedAt":"2020-09-30T12:34:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The append_signoff() function takes an \"ignore_footer\"\nargument, which specifies a number of bytes at the end of\nthe message buffer which should not be considered (they\ncannot contain trailers, and the trailer is spliced in\nbefore them).\n\nBut to find the existing trailers, it calls into\nhas_conforming_trailer(). That function takes an\nignore_footer parameter, but since 967dfd4d56 (sequencer:\nuse trailer's trailer layout, 2016-11-02) the parameter is\ncompletely ignored.\n\nThe trailer interface we're using takes a single string,\nwith no option to tell it to use part of the string.\nHowever, since we have a mutable strbuf, we can work around\nthis by simply overwriting (and later restoring) the\nboundary with a NUL.\n\nI'm not sure if this can actually trigger a bug in practice.\nIt's easy to get a non-zero ignore_footer by doing something\nlike this:\n\n git commit -F - --cleanup=verbatim <<-EOF\n subject\n\n body\n\n Signed-off-by: me\n\n # this looks like a comment, but is actually in the\n # message! That makes the earlier s-o-b fake.\n EOF\n\n git commit --amend -s\n\nThere git-commit calls ignore_non_trailer() to count up the\n\"#\" cruft, which becomes the ignore_footer header. But it\nworks even without this patch! That's because the trailer\ncode _also_ calls ignore_non_trailer() and skips the cruft,\ntoo. So it happens to work because the only callers with a\nnon-zero ignore_footer are using the exact same function\nthat the trailer parser uses internally.\n\nAnd that seems true for all of the current callers, but\nthere's nothing guaranteeing it. We're better off only\nfeeding the correct buffer to the trailer code in the first\nplace.\n\nSigned-off-by: Jeff King \n---\nThis was actually posted as part of an earlier series:\n\n https://lore.kernel.org/git/20180824072629.GA11977@sigill.intra.peff.net/\n\nIt ended as:\n\n I think patch 9 is not hurting anything and may later help us, but I\n could take or leave it.\n\nand we left it. But now I have the ulterior motive of wanting to get rid\nof the unused parameter warning. :) Another option would be to leave the\ncode as-is and just drop the ignored parameter. But I think this is\nprobably a safe and reasonable patch (I've been running with it in my\npersonal fork for 2 years, though perhaps I don't really exercise the\ntrailer code's corner cases myself).\n\n sequencer.c | 9 +++++++++\n 1 file changed, 9 insertions(+)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 6e9aabaac1..e454264fbc 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -249,11 +249,20 @@ static int has_conforming_footer(struct strbuf *sb, struct strbuf *sob,\n \tstruct trailer_info info;\n \tsize_t i;\n \tint found_sob = 0, found_sob_last = 0;\n+\tchar saved_char;\n \n \topts.no_divider = 1;\n \n+\tif (ignore_footer) {\n+\t\tsaved_char = sb->buf[sb->len - ignore_footer];\n+\t\tsb->buf[sb->len - ignore_footer] = '\\0';\n+\t}\n+\n \ttrailer_info_get(&info, sb->buf, &opts);\n \n+\tif (ignore_footer)\n+\t\tsb->buf[sb->len - ignore_footer] = saved_char;\n+\n \tif (info.trailer_start == info.trailer_end)\n \t\treturn 0;\n \n-- \n2.28.0.1173.gad90222cf0\n\n"},{"id":"406676","messageId":"20200930123500.GJ1901279@coredump.intra.peff.net","threadId":"54328","inReplyTo":"20200930122732.GA1901036@coredump.intra.peff.net","subject":"[PATCH 10/10] dir.c: drop unused \"untracked\" from treat_path_fast()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-09-30T12:35:00Z","receivedAt":"2020-09-30T12:35:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We don't use the untracked_cache_dir parameter that is passed in, but\ninstead look at the untracked_cache_dir inside the cached_dir struct we\nare passed. It's been this way since the introduction of\ntreat_path_fast() in 91a2288b5f (untracked cache: record/validate dir\nmtime and reuse cached output, 2015-03-08).\n\nSigned-off-by: Jeff King \n---\nThis is the one I'm least sure of (not that it makes anything worse, but\nthat it might be hiding a bug; I'm pretty sure I just don't understand\nhow the untracked-cache code works, though).\n\n dir.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 3018a657b0..78387110e6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2105,7 +2105,6 @@ static int resolve_dtype(int dtype, struct index_state *istate,\n }\n \n static enum path_treatment treat_path_fast(struct dir_struct *dir,\n-\t\t\t\t\t struct untracked_cache_dir *untracked,\n \t\t\t\t\t struct cached_dir *cdir,\n \t\t\t\t\t struct index_state *istate,\n \t\t\t\t\t struct strbuf *path,\n@@ -2153,7 +2152,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n \tint has_path_in_index, dtype, excluded;\n \n \tif (!cdir->d_name)\n-\t\treturn treat_path_fast(dir, untracked, cdir, istate, path,\n+\t\treturn treat_path_fast(dir, cdir, istate, path,\n \t\t\t\t baselen, pathspec);\n \tif (is_dot_or_dotdot(cdir->d_name) || !fspathcmp(cdir->d_name, \".git\"))\n \t\treturn path_none;\n-- \n2.28.0.1173.gad90222cf0\n"}]}