{"thread":{"id":"50786","subject":"[PATCH 0/13] more unused parameter cleanups","startedAt":"2019-03-20T08:13:02Z","lastAt":"2019-03-21T09:44:18Z","messageCount":19,"participants":["Jeff King","Junio C Hamano","Martin Ågren","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":13},"messages":[{"id":"371956","messageId":"20190320081258.GA5621@sigill.intra.peff.net","threadId":"50786","inReplyTo":null,"subject":"[PATCH 0/13] more unused parameter cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:12:58Z","receivedAt":"2019-03-20T08:13:02Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's another round of -Wunused-parameter cleanups. Previous rounds\nwere at [1] and [2]. As before, these are mostly just removals, so it's\neasy to see there's no behavior change (there are a couple of cleanups\nthat ride along, though, so watch for those).\n\nThere are two minor conflicts when merging with pu:\n\n  - jt/fetch-cdn-offload tweaked the \"pack_lockfile\" parameter to\n    fetch_pack(). A few other parameters are dropped in this series.\n    The textual resolution is pretty straightforward.\n\n  - ps/stash-in-c (et al) added a new call to report_path_error() in\n    builtin/stash.c, which here loses its redundant \"prefix\" parameter.\n    There's no textual conflict, but the new call needs to drop its\n    final NULL parameter in order to compile.\n\n[1] https://public-inbox.org/git/20190214054736.GA20091@sigill.intra.peff.net/\n[2] https://public-inbox.org/git/20190124131104.GA24017@sigill.intra.peff.net/\n\nThe patches are:\n\n  [01/13]: revision: drop some unused \"revs\" parameters\n  [02/13]: log: drop unused rev_info from early output\n  [03/13]: log: drop unused \"len\" from show_tagger()\n  [04/13]: update-index: drop unused prefix_length parameter from do_reupdate()\n  [05/13]: test-date: drop unused \"now\" parameter from parse_dates()\n  [06/13]: unpack-trees: drop name_entry from traverse_by_cache_tree()\n  [07/13]: unpack-trees: drop unused error_type parameters\n  [08/13]: report_path_error(): drop unused prefix parameter\n  [09/13]: fetch_pack(): drop unused parameters\n  [10/13]: parse-options: drop unused ctx parameter from show_gitcomp()\n  [11/13]: pretty: drop unused \"type\" parameter in needs_rfc2047_encoding()\n  [12/13]: pretty: drop unused strbuf from parse_padding_placeholder()\n  [13/13]: parse_opt_ref_sorting: always use with NONEG flag\n\n builtin/branch.c            |  3 +--\n builtin/checkout.c          |  2 +-\n builtin/commit.c            |  6 +++---\n builtin/fetch-pack.c        |  2 +-\n builtin/for-each-ref.c      |  3 +--\n builtin/log.c               | 18 +++++++++---------\n builtin/ls-files.c          |  2 +-\n builtin/ls-remote.c         |  3 +--\n builtin/submodule--helper.c |  2 +-\n builtin/tag.c               |  3 +--\n builtin/update-index.c      |  5 ++---\n dir.c                       |  3 +--\n dir.h                       |  2 +-\n fetch-pack.c                |  3 +--\n fetch-pack.h                |  3 +--\n parse-options.c             |  5 ++---\n pretty.c                    | 12 +++++-------\n ref-filter.c                |  9 +++++++--\n ref-filter.h                |  5 +++++\n revision.c                  | 12 ++++++------\n t/helper/test-date.c        |  4 ++--\n transport.c                 | 10 ++++------\n unpack-trees.c              |  9 +++------\n 23 files changed, 60 insertions(+), 66 deletions(-)\n\n-Peff\n"},{"id":"371957","messageId":"20190320081336.GA10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 01/13] revision: drop some unused \"revs\" parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:13:36Z","receivedAt":"2019-03-20T08:13:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are several internal helpers that take a rev_info struct but don't\nactually look at it. While one could argue that all helpers in\nrevision.c should take a rev_info struct for consistency, dropping the\nunused parameter makes it clear that they don't actually depend on any\nother rev options.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n revision.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex eb8e51bc63..695021d6fc 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1894,7 +1894,7 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi\n \treturn 0;\n }\n \n-static void read_pathspec_from_stdin(struct rev_info *revs, struct strbuf *sb,\n+static void read_pathspec_from_stdin(struct strbuf *sb,\n \t\t\t\t     struct argv_array *prune)\n {\n \twhile (strbuf_getline(sb, stdin) != EOF)\n@@ -1928,7 +1928,7 @@ static void read_revisions_from_stdin(struct rev_info *revs,\n \t\t\tdie(\"bad revision '%s'\", sb.buf);\n \t}\n \tif (seen_dashdash)\n-\t\tread_pathspec_from_stdin(revs, &sb, prune);\n+\t\tread_pathspec_from_stdin(&sb, prune);\n \n \tstrbuf_release(&sb);\n \twarn_on_object_refname_ambiguity = save_warning;\n@@ -2748,7 +2748,7 @@ static struct merge_simplify_state *locate_simplify_state(struct rev_info *revs,\n \treturn st;\n }\n \n-static int mark_redundant_parents(struct rev_info *revs, struct commit *commit)\n+static int mark_redundant_parents(struct commit *commit)\n {\n \tstruct commit_list *h = reduce_heads(commit->parents);\n \tint i = 0, marked = 0;\n@@ -2784,7 +2784,7 @@ static int mark_redundant_parents(struct rev_info *revs, struct commit *commit)\n \treturn marked;\n }\n \n-static int mark_treesame_root_parents(struct rev_info *revs, struct commit *commit)\n+static int mark_treesame_root_parents(struct commit *commit)\n {\n \tstruct commit_list *p;\n \tint marked = 0;\n@@ -2976,8 +2976,8 @@ static struct commit_list **simplify_one(struct rev_info *revs, struct commit *c\n \t * Detect and simplify both cases.\n \t */\n \tif (1 < cnt) {\n-\t\tint marked = mark_redundant_parents(revs, commit);\n-\t\tmarked += mark_treesame_root_parents(revs, commit);\n+\t\tint marked = mark_redundant_parents(commit);\n+\t\tmarked += mark_treesame_root_parents(commit);\n \t\tif (marked)\n \t\t\tmarked -= leave_one_treesame_to_parent(revs, commit);\n \t\tif (marked)\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371958","messageId":"20190320081342.GB10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 02/13] log: drop unused rev_info from early output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:13:42Z","receivedAt":"2019-03-20T08:13:45Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The early output code passes around a rev_info struct but doesn't need\nit. The setup step only turns on global signal handlers, and the\n\"estimate\" step is done completely from the rev->commits list that is\npassed in separately.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/log.c | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex ab859f5904..6595471ddf 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -251,7 +251,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n  * This gives a rough estimate for how many commits we\n  * will print out in the list.\n  */\n-static int estimate_commit_count(struct rev_info *rev, struct commit_list *list)\n+static int estimate_commit_count(struct commit_list *list)\n {\n \tint n = 0;\n \n@@ -289,7 +289,7 @@ static void log_show_early(struct rev_info *revs, struct commit_list *list)\n \t\tswitch (simplify_commit(revs, commit)) {\n \t\tcase commit_show:\n \t\t\tif (show_header) {\n-\t\t\t\tint n = estimate_commit_count(revs, list);\n+\t\t\t\tint n = estimate_commit_count(list);\n \t\t\t\tshow_early_header(revs, \"incomplete\", n);\n \t\t\t\tshow_header = 0;\n \t\t\t}\n@@ -333,7 +333,7 @@ static void early_output(int signal)\n \tshow_early_output = log_show_early;\n }\n \n-static void setup_early_output(struct rev_info *rev)\n+static void setup_early_output(void)\n {\n \tstruct sigaction sa;\n \n@@ -364,7 +364,7 @@ static void setup_early_output(struct rev_info *rev)\n \n static void finish_early_output(struct rev_info *rev)\n {\n-\tint n = estimate_commit_count(rev, rev->commits);\n+\tint n = estimate_commit_count(rev->commits);\n \tsignal(SIGALRM, SIG_IGN);\n \tshow_early_header(rev, \"done\", n);\n }\n@@ -376,7 +376,7 @@ static int cmd_log_walk(struct rev_info *rev)\n \tint saved_dcctc = 0, close_file = rev->diffopt.close_file;\n \n \tif (rev->early_output)\n-\t\tsetup_early_output(rev);\n+\t\tsetup_early_output();\n \n \tif (prepare_revision_walk(rev))\n \t\tdie(_(\"revision walk setup failed\"));\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371959","messageId":"20190320081400.GC10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 03/13] log: drop unused \"len\" from show_tagger()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:14:01Z","receivedAt":"2019-03-20T08:14:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We pass the length of the found \"tagger\" line to show_tagger(), but it\ndoes not use it; instead, it passes the string to pp_user_info(), which\nreads until newline or NUL. This is OK for our purposes because we\nalways read the object contents into a buffer with an extra NUL (and\nindeed, our sole caller already relies on this by using starts_with).\n\nLet's drop the ignored parameter. And while we're touching the caller,\nlet's use skip_prefix() to avoid a magic number.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/log.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 6595471ddf..35314d12ec 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -490,7 +490,7 @@ int cmd_whatchanged(int argc, const char **argv, const char *prefix)\n \treturn cmd_log_walk(&rev);\n }\n \n-static void show_tagger(char *buf, int len, struct rev_info *rev)\n+static void show_tagger(const char *buf, struct rev_info *rev)\n {\n \tstruct strbuf out = STRBUF_INIT;\n \tstruct pretty_print_context pp = {0};\n@@ -546,11 +546,11 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev)\n \tassert(type == OBJ_TAG);\n \twhile (offset < size && buf[offset] != '\\n') {\n \t\tint new_offset = offset + 1;\n+\t\tconst char *ident;\n \t\twhile (new_offset < size && buf[new_offset++] != '\\n')\n \t\t\t; /* do nothing */\n-\t\tif (starts_with(buf + offset, \"tagger \"))\n-\t\t\tshow_tagger(buf + offset + 7,\n-\t\t\t\t    new_offset - offset - 7, rev);\n+\t\tif (skip_prefix(buf + offset, \"tagger \", &ident))\n+\t\t\tshow_tagger(ident, rev);\n \t\toffset = new_offset;\n \t}\n \n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371960","messageId":"20190320081407.GD10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 04/13] update-index: drop unused prefix_length parameter from do_reupdate()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:14:07Z","receivedAt":"2019-03-20T08:14:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The prefix is always a NUL-terminated string, and we just end up passing\nit along to parse_pathspec() anyway (which does not even take a length).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/update-index.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/update-index.c b/builtin/update-index.c\nindex 1b6c42f748..ff5cfd1194 100644\n--- a/builtin/update-index.c\n+++ b/builtin/update-index.c\n@@ -724,7 +724,7 @@ static int do_unresolve(int ac, const char **av,\n }\n \n static int do_reupdate(int ac, const char **av,\n-\t\t       const char *prefix, int prefix_length)\n+\t\t       const char *prefix)\n {\n \t/* Read HEAD and run update-index on paths that are\n \t * merged and already different between index and HEAD.\n@@ -940,8 +940,7 @@ static enum parse_opt_result reupdate_callback(\n \n \t/* consume remaining arguments. */\n \tsetup_work_tree();\n-\t*has_errors = do_reupdate(ctx->argc, ctx->argv,\n-\t\t\t\tprefix, prefix ? strlen(prefix) : 0);\n+\t*has_errors = do_reupdate(ctx->argc, ctx->argv, prefix);\n \tif (*has_errors)\n \t\tactive_cache_changed = 0;\n \n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371961","messageId":"20190320081430.GE10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 05/13] test-date: drop unused \"now\" parameter from parse_dates()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:14:30Z","receivedAt":"2019-03-20T08:14:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We only need the current time for relative dates like \"5\nminutes ago\", and those are parsed only through approxidate,\nnot the strict parser used by parse_dates().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/helper/test-date.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-date.c b/t/helper/test-date.c\nindex b3253803ac..585347ea48 100644\n--- a/t/helper/test-date.c\n+++ b/t/helper/test-date.c\n@@ -55,7 +55,7 @@ static void show_dates(const char **argv, const char *format)\n \t}\n }\n \n-static void parse_dates(const char **argv, struct timeval *now)\n+static void parse_dates(const char **argv)\n {\n \tstruct strbuf result = STRBUF_INIT;\n \n@@ -124,7 +124,7 @@ int cmd__date(int argc, const char **argv)\n \telse if (skip_prefix(*argv, \"show:\", &x))\n \t\tshow_dates(argv+1, x);\n \telse if (!strcmp(*argv, \"parse\"))\n-\t\tparse_dates(argv+1, &now);\n+\t\tparse_dates(argv+1);\n \telse if (!strcmp(*argv, \"approxidate\"))\n \t\tparse_approxidate(argv+1, &now);\n \telse if (!strcmp(*argv, \"timestamp\"))\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371962","messageId":"20190320081507.GF10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 06/13] unpack-trees: drop name_entry from traverse_by_cache_tree()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:15:07Z","receivedAt":"2019-03-20T08:15:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We pull the names from the existing index rather than the tree entry,\nwhich is after all the point of this function. Let's drop the unused\n\"names\" parameter.\n\nNote that we leave the \"nr_names\" parameter, as it tells us how many\ntrees we are traversing (and thus how many index stages to set up).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n unpack-trees.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 22c41a3ba8..79551ab9cc 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -707,7 +707,6 @@ static int index_pos_by_traverse_info(struct name_entry *names,\n  * instead of ODB since we already know what these trees contain.\n  */\n static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names,\n-\t\t\t\t  struct name_entry *names,\n \t\t\t\t  struct traverse_info *info)\n {\n \tstruct cache_entry *src[MAX_UNPACK_TREES + 1] = { NULL, };\n@@ -797,7 +796,7 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,\n \t\t * unprocessed entries before 'pos'.\n \t\t */\n \t\tbottom = o->cache_bottom;\n-\t\tret = traverse_by_cache_tree(pos, nr_entries, n, names, info);\n+\t\tret = traverse_by_cache_tree(pos, nr_entries, n, info);\n \t\to->cache_bottom = bottom;\n \t\treturn ret;\n \t}\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371963","messageId":"20190320081527.GG10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 07/13] unpack-trees: drop unused error_type parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:15:27Z","receivedAt":"2019-03-20T08:15:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The verify_clean_subdirectory() helper takes an error_type parameter\nfrom the caller, but doesn't actually use it. Instead, when it calls\nadd_rejected_path() it passes NOT_UPTODATE_DIR, its own custom error\ntype which is more specific than what the caller provides. Likewise for\nverify_clean_submodule(), which always passes WOULD_LOSE_SUBMODULE.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n unpack-trees.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 79551ab9cc..5f65ff5804 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1758,7 +1758,6 @@ static void invalidate_ce_path(const struct cache_entry *ce,\n  */\n static int verify_clean_submodule(const char *old_sha1,\n \t\t\t\t  const struct cache_entry *ce,\n-\t\t\t\t  enum unpack_trees_error_types error_type,\n \t\t\t\t  struct unpack_trees_options *o)\n {\n \tif (!submodule_from_ce(ce))\n@@ -1769,7 +1768,6 @@ static int verify_clean_submodule(const char *old_sha1,\n }\n \n static int verify_clean_subdirectory(const struct cache_entry *ce,\n-\t\t\t\t     enum unpack_trees_error_types error_type,\n \t\t\t\t     struct unpack_trees_options *o)\n {\n \t/*\n@@ -1792,7 +1790,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,\n \t\tif (!sub_head && oideq(&oid, &ce->oid))\n \t\t\treturn 0;\n \t\treturn verify_clean_submodule(sub_head ? NULL : oid_to_hex(&oid),\n-\t\t\t\t\t      ce, error_type, o);\n+\t\t\t\t\t      ce, o);\n \t}\n \n \t/*\n@@ -1888,7 +1886,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,\n \t\t * files that are in \"foo/\" we would lose\n \t\t * them.\n \t\t */\n-\t\tif (verify_clean_subdirectory(ce, error_type, o) < 0)\n+\t\tif (verify_clean_subdirectory(ce, o) < 0)\n \t\t\treturn -1;\n \t\treturn 0;\n \t}\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371964","messageId":"20190320081548.GH10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 08/13] report_path_error(): drop unused prefix parameter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:15:48Z","receivedAt":"2019-03-20T08:15:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This hasn't been used since 17ddc66e70 (convert report_path_error to\ntake struct pathspec, 2013-07-14), as the names in the struct will have\nalready been prefixed when they were parsed.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/checkout.c          | 2 +-\n builtin/commit.c            | 6 +++---\n builtin/ls-files.c          | 2 +-\n builtin/submodule--helper.c | 2 +-\n dir.c                       | 3 +--\n dir.h                       | 2 +-\n 6 files changed, 8 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 0e6037b296..72f7110fd8 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -376,7 +376,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \t\t\t\t\t\t\tps_matched,\n \t\t\t\t\t\t\topts);\n \n-\tif (report_path_error(ps_matched, &opts->pathspec, opts->prefix)) {\n+\tif (report_path_error(ps_matched, &opts->pathspec)) {\n \t\tfree(ps_matched);\n \t\treturn 1;\n \t}\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex f17537474a..a138ff85b0 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -235,7 +235,7 @@ static int commit_index_files(void)\n  * and return the paths that match the given pattern in list.\n  */\n static int list_paths(struct string_list *list, const char *with_tree,\n-\t\t      const char *prefix, const struct pathspec *pattern)\n+\t\t      const struct pathspec *pattern)\n {\n \tint i, ret;\n \tchar *m;\n@@ -264,7 +264,7 @@ static int list_paths(struct string_list *list, const char *with_tree,\n \t\t\titem->util = item; /* better a valid pointer than a fake one */\n \t}\n \n-\tret = report_path_error(m, pattern, prefix);\n+\tret = report_path_error(m, pattern);\n \tfree(m);\n \treturn ret;\n }\n@@ -454,7 +454,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix\n \t\t\tdie(_(\"cannot do a partial commit during a cherry-pick.\"));\n \t}\n \n-\tif (list_paths(&partial, !current_head ? NULL : \"HEAD\", prefix, &pathspec))\n+\tif (list_paths(&partial, !current_head ? NULL : \"HEAD\", &pathspec))\n \t\texit(1);\n \n \tdiscard_cache();\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 29a8762d46..37a08c714b 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -680,7 +680,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \n \tif (ps_matched) {\n \t\tint bad;\n-\t\tbad = report_path_error(ps_matched, &pathspec, prefix);\n+\t\tbad = report_path_error(ps_matched, &pathspec);\n \t\tif (bad)\n \t\t\tfprintf(stderr, \"Did you forget to 'git add'?\\n\");\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6bcc4f1bd7..8342b78add 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -348,7 +348,7 @@ static int module_list_compute(int argc, const char **argv,\n \t\t\ti++;\n \t}\n \n-\tif (ps_matched && report_path_error(ps_matched, pathspec, prefix))\n+\tif (ps_matched && report_path_error(ps_matched, pathspec))\n \t\tresult = -1;\n \n \tfree(ps_matched);\ndiff --git a/dir.c b/dir.c\nindex b2cabadf25..5286f710cc 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -502,8 +502,7 @@ int submodule_path_match(const struct index_state *istate,\n }\n \n int report_path_error(const char *ps_matched,\n-\t\t      const struct pathspec *pathspec,\n-\t\t      const char *prefix)\n+\t\t      const struct pathspec *pathspec)\n {\n \t/*\n \t * Make sure all pathspec matched; otherwise it is an error.\ndiff --git a/dir.h b/dir.h\nindex e3ec26143d..823bae628b 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -220,7 +220,7 @@ extern int match_pathspec(const struct index_state *istate,\n \t\t\t  const struct pathspec *pathspec,\n \t\t\t  const char *name, int namelen,\n \t\t\t  int prefix, char *seen, int is_dir);\n-extern int report_path_error(const char *ps_matched, const struct pathspec *pathspec, const char *prefix);\n+extern int report_path_error(const char *ps_matched, const struct pathspec *pathspec);\n extern int within_depth(const char *name, int namelen, int depth, int max_depth);\n \n extern int fill_directory(struct dir_struct *dir,\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371965","messageId":"20190320081614.GI10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 09/13] fetch_pack(): drop unused parameters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:16:14Z","receivedAt":"2019-03-20T08:16:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We don't need the caller of fetch_pack() to pass in \"dest\", which is the\nremote URL. Since ba227857d2 (Reduce the number of connects when\nfetching, 2008-02-04), the caller is responsible for calling\ngit_connect() itself, and our \"dest\" parameter is unused.\n\nThat commit also started passing us the resulting \"conn\" child_process\nfrom git_connect(). But likewise, we do not need do anything with it.\nThe descriptors in \"fd\" are enough for us, and the caller is responsible\nfor cleaning up \"conn\".\n\nWe can just drop both parameters.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/fetch-pack.c |  2 +-\n fetch-pack.c         |  3 +--\n fetch-pack.h         |  3 +--\n transport.c          | 10 ++++------\n 4 files changed, 7 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 153a2bd282..dc1485c8aa 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -234,7 +234,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\tBUG(\"unknown protocol version\");\n \t}\n \n-\tref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,\n+\tref = fetch_pack(&args, fd, ref, sought, nr_sought,\n \t\t\t &shallow, pack_lockfile_ptr, version);\n \tif (pack_lockfile) {\n \t\tprintf(\"lock %s\\n\", pack_lockfile);\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex e69993b2eb..8d67d4e362 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -1615,9 +1615,8 @@ static int iterate_ref_map(void *cb_data, struct object_id *oid)\n }\n \n struct ref *fetch_pack(struct fetch_pack_args *args,\n-\t\t       int fd[], struct child_process *conn,\n+\t\t       int fd[],\n \t\t       const struct ref *ref,\n-\t\t       const char *dest,\n \t\t       struct ref **sought, int nr_sought,\n \t\t       struct oid_array *shallow,\n \t\t       char **pack_lockfile,\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex 43ec344d95..67f684229a 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -78,9 +78,8 @@ struct fetch_pack_args {\n  * marked as such.\n  */\n struct ref *fetch_pack(struct fetch_pack_args *args,\n-\t\t       int fd[], struct child_process *conn,\n+\t\t       int fd[],\n \t\t       const struct ref *ref,\n-\t\t       const char *dest,\n \t\t       struct ref **sought,\n \t\t       int nr_sought,\n \t\t       struct oid_array *shallow,\ndiff --git a/transport.c b/transport.c\nindex d0608df5c9..365ea574c7 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -314,7 +314,6 @@ static int fetch_refs_via_pack(struct transport *transport,\n \tint ret = 0;\n \tstruct git_transport_data *data = transport->data;\n \tstruct ref *refs = NULL;\n-\tchar *dest = xstrdup(transport->url);\n \tstruct fetch_pack_args args;\n \tstruct ref *refs_tmp = NULL;\n \n@@ -356,16 +355,16 @@ static int fetch_refs_via_pack(struct transport *transport,\n \n \tswitch (data->version) {\n \tcase protocol_v2:\n-\t\trefs = fetch_pack(&args, data->fd, data->conn,\n+\t\trefs = fetch_pack(&args, data->fd,\n \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n-\t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n+\t\t\t\t  to_fetch, nr_heads, &data->shallow,\n \t\t\t\t  &transport->pack_lockfile, data->version);\n \t\tbreak;\n \tcase protocol_v1:\n \tcase protocol_v0:\n-\t\trefs = fetch_pack(&args, data->fd, data->conn,\n+\t\trefs = fetch_pack(&args, data->fd,\n \t\t\t\t  refs_tmp ? refs_tmp : transport->remote_refs,\n-\t\t\t\t  dest, to_fetch, nr_heads, &data->shallow,\n+\t\t\t\t  to_fetch, nr_heads, &data->shallow,\n \t\t\t\t  &transport->pack_lockfile, data->version);\n \t\tbreak;\n \tcase protocol_unknown_version:\n@@ -389,7 +388,6 @@ static int fetch_refs_via_pack(struct transport *transport,\n \n \tfree_refs(refs_tmp);\n \tfree_refs(refs);\n-\tfree(dest);\n \treturn ret;\n }\n \n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371966","messageId":"20190320081627.GJ10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 10/13] parse-options: drop unused ctx parameter from show_gitcomp()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:16:27Z","receivedAt":"2019-03-20T08:16:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The completion display doesn't actually care about where we are in the\nparsing. It's generated completely from the set of available options. So\nwe don't need to see the parse-options context struct at all.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n parse-options.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex cec74522e5..ade83a7b20 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -523,8 +523,7 @@ static void show_negated_gitcomp(const struct option *opts, int nr_noopts)\n \t}\n }\n \n-static int show_gitcomp(struct parse_opt_ctx_t *ctx,\n-\t\t\tconst struct option *opts)\n+static int show_gitcomp(const struct option *opts)\n {\n \tconst struct option *original_opts = opts;\n \tint nr_noopts = 0;\n@@ -603,7 +602,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,\n \n \t\t/* lone --git-completion-helper is asked by git-completion.bash */\n \t\tif (ctx->total == 1 && !strcmp(arg + 1, \"-git-completion-helper\"))\n-\t\t\treturn show_gitcomp(ctx, options);\n+\t\t\treturn show_gitcomp(options);\n \n \t\tif (arg[1] != '-') {\n \t\t\tctx->opt = arg + 1;\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371967","messageId":"20190320081636.GK10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 11/13] pretty: drop unused \"type\" parameter in needs_rfc2047_encoding()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:16:36Z","receivedAt":"2019-03-20T08:16:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"should we encode\" check was split off from add_rfc2047() into its\nown function in 41dd00bad3 (format-patch: fix rfc2047 address encoding\nwith respect to rfc822 specials, 2012-10-18). But only the \"add\" half\nneeds to know the rfc2047_type, since it only affects _how_ we encode,\nnot whether we do.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pretty.c | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex f496f0f128..f925a014f9 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -343,8 +343,7 @@ static int is_rfc2047_special(char ch, enum rfc2047_type type)\n \treturn !(isalnum(ch) || ch == '!' || ch == '*' || ch == '+' || ch == '-' || ch == '/');\n }\n \n-static int needs_rfc2047_encoding(const char *line, int len,\n-\t\t\t\t  enum rfc2047_type type)\n+static int needs_rfc2047_encoding(const char *line, int len)\n {\n \tint i;\n \n@@ -470,7 +469,7 @@ void pp_user_info(struct pretty_print_context *pp,\n \t\t}\n \n \t\tstrbuf_addstr(sb, \"From: \");\n-\t\tif (needs_rfc2047_encoding(namebuf, namelen, RFC2047_ADDRESS)) {\n+\t\tif (needs_rfc2047_encoding(namebuf, namelen)) {\n \t\t\tadd_rfc2047(sb, namebuf, namelen,\n \t\t\t\t    encoding, RFC2047_ADDRESS);\n \t\t\tmax_length = 76; /* per rfc2047 */\n@@ -1728,7 +1727,7 @@ void pp_title_line(struct pretty_print_context *pp,\n \tif (pp->print_email_subject) {\n \t\tif (pp->rev)\n \t\t\tfmt_output_email_subject(sb, pp->rev);\n-\t\tif (needs_rfc2047_encoding(title.buf, title.len, RFC2047_SUBJECT))\n+\t\tif (needs_rfc2047_encoding(title.buf, title.len))\n \t\t\tadd_rfc2047(sb, title.buf, title.len,\n \t\t\t\t\t\tencoding, RFC2047_SUBJECT);\n \t\telse\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371968","messageId":"20190320081642.GL10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 12/13] pretty: drop unused strbuf from parse_padding_placeholder()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:16:42Z","receivedAt":"2019-03-20T08:16:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Unlike other parts of the --pretty user-format expansion,\nthis function is not actually writing to the output, but\ninstead just storing the padding values into a context\nstruct. We don't need to be passed a strbuf at all.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pretty.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex f925a014f9..ced0485257 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -988,8 +988,7 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 */\n \treturn rest - placeholder;\n }\n \n-static size_t parse_padding_placeholder(struct strbuf *sb,\n-\t\t\t\t\tconst char *placeholder,\n+static size_t parse_padding_placeholder(const char *placeholder,\n \t\t\t\t\tstruct format_commit_context *c)\n {\n \tconst char *ch = placeholder;\n@@ -1194,7 +1193,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */\n \n \tcase '<':\n \tcase '>':\n-\t\treturn parse_padding_placeholder(sb, placeholder, c);\n+\t\treturn parse_padding_placeholder(placeholder, c);\n \t}\n \n \t/* these depend on the commit */\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"371969","messageId":"20190320081650.GM10403@sigill.intra.peff.net","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"[PATCH 13/13] parse_opt_ref_sorting: always use with NONEG flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T08:16:51Z","receivedAt":"2019-03-20T08:16:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The \"--sort\" parameter of for-each-ref, etc, does not handle negation,\nand instead returns an error to the parse-options code. But neither\npiece of code prints anything for the user, which may leave them\nconfused:\n\n  $ git for-each-ref --no-sort\n  $ echo $?\n  129\n\nAs the comment in the callback function notes, this probably should\nclear the list, which would make it consistent with other list-like\noptions (i.e., anything that uses OPT_STRING_LIST currently).\nUnfortunately that's a bit tricky due to the way the ref-filter code\nworks. But in the meantime, let's at least make the error a little less\nconfusing:\n\n  - switch to using PARSE_OPT_NONEG in the option definition, which will\n    cause the options code to produce a useful message\n\n  - since this was cut-and-pasted to four different spots, let's define\n    a single OPT_REF_SORT() macro that we can use everywhere\n\n  - the callback can use BUG_ON_OPT_NEG() to make sure the correct flags\n    are used (incidentally, this also satisfies -Wunused-parameters,\n    since we're now looking at \"unset\")\n\n  - expand the comment into a NEEDSWORK to make it clear that the\n    direction is right, but the details need to be worked out\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/branch.c       | 3 +--\n builtin/for-each-ref.c | 3 +--\n builtin/ls-remote.c    | 3 +--\n builtin/tag.c          | 3 +--\n ref-filter.c           | 9 +++++++--\n ref-filter.h           | 5 +++++\n 6 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 4c83055730..d4359b33ac 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -644,8 +644,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_MERGED(&filter, N_(\"print only branches that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only branches that are not merged\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"list branches in columns\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only branches of the object\"), 0, parse_opt_object_name\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex e931be9ce4..465153e853 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -37,8 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER( 0 , \"count\", &maxcount, N_(\"show only <n> matched refs\")),\n \t\tOPT_STRING(  0 , \"format\", &format.format, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT__COLOR(&format.use_color, N_(\"respect format colors\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t    N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\tOPT_CALLBACK(0, \"points-at\", &filter.points_at,\n \t\t\t     N_(\"object\"), N_(\"print only refs which points at the given object\"),\n \t\t\t     parse_opt_object_name),\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 1d7f1f5ce2..6ef519514b 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -67,8 +67,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"refs\", &flags, N_(\"do not show peeled tags\"), REF_NORMAL),\n \t\tOPT_BOOL(0, \"get-url\", &get_url,\n \t\t\t N_(\"take url.<base>.insteadOf into account\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\tOPT_SET_INT_F(0, \"exit-code\", &status,\n \t\t\t      N_(\"exit with exit code 2 if no matching refs are found\"),\n \t\t\t      2, PARSE_OPT_NOCOMPLETE),\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 02f6bd1279..ad97595fbf 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -412,8 +412,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), PARSE_OPT_LASTARG_DEFAULT,\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3aca105307..8d11a94cbd 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -2337,8 +2337,13 @@ void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)\n \n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n {\n-\tif (!arg) /* should --no-sort void the list ? */\n-\t\treturn -1;\n+\t/*\n+\t * NEEDSWORK: We should probably clear the list in this case, but we've\n+\t * already munged the global used_atoms list, which would need to be\n+\t * undone.\n+\t */\n+\tBUG_ON_OPT_NEG(unset);\n+\n \tparse_ref_sorting(opt->value, arg);\n \treturn 0;\n }\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 85c8ebc3b9..5ba46a7e38 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -96,6 +96,11 @@ struct ref_format {\n #define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED(\"merged\", f, h)\n #define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED(\"no-merged\", f, h)\n \n+#define OPT_REF_SORT(var) \\\n+\tOPT_CALLBACK_F(0, \"sort\", (var), \\\n+\t\t       N_(\"key\"), N_(\"field name to sort\"), \\\n+\t\t       PARSE_OPT_NONEG, parse_opt_ref_sorting)\n+\n /*\n  * API for filtering a set of refs. Based on the type of refs the user\n  * has requested, we iterate through those refs and apply filters\n-- \n2.21.0.701.g4401309e11\n"},{"id":"371970","messageId":"xmqqr2b1vqq4.fsf@gitster-ct.c.googlers.com","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"Re: [PATCH 0/13] more unused parameter cleanups","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-03-20T09:29:23Z","receivedAt":"2019-03-20T09:29:30Z","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> Here's another round of -Wunused-parameter cleanups. Previous rounds\n> were at [1] and [2]. As before, these are mostly just removals, so it's\n> easy to see there's no behavior change (there are a couple of cleanups\n> that ride along, though, so watch for those).\n>\n> There are two minor conflicts when merging with pu:\n>\n>   - jt/fetch-cdn-offload tweaked the \"pack_lockfile\" parameter to\n>     fetch_pack(). A few other parameters are dropped in this series.\n>     The textual resolution is pretty straightforward.\n>\n>   - ps/stash-in-c (et al) added a new call to report_path_error() in\n>     builtin/stash.c, which here loses its redundant \"prefix\" parameter.\n>     There's no textual conflict, but the new call needs to drop its\n>     final NULL parameter in order to compile.\n>\n> [1] https://public-inbox.org/git/20190214054736.GA20091@sigill.intra.peff.net/\n> [2] https://public-inbox.org/git/20190124131104.GA24017@sigill.intra.peff.net/\n>\n> The patches are:\n>\n>   [01/13]: revision: drop some unused \"revs\" parameters\n>   [02/13]: log: drop unused rev_info from early output\n>   [03/13]: log: drop unused \"len\" from show_tagger()\n>   [04/13]: update-index: drop unused prefix_length parameter from do_reupdate()\n>   [05/13]: test-date: drop unused \"now\" parameter from parse_dates()\n>   [06/13]: unpack-trees: drop name_entry from traverse_by_cache_tree()\n>   [07/13]: unpack-trees: drop unused error_type parameters\n>   [08/13]: report_path_error(): drop unused prefix parameter\n>   [09/13]: fetch_pack(): drop unused parameters\n>   [10/13]: parse-options: drop unused ctx parameter from show_gitcomp()\n>   [11/13]: pretty: drop unused \"type\" parameter in needs_rfc2047_encoding()\n>   [12/13]: pretty: drop unused strbuf from parse_padding_placeholder()\n>   [13/13]: parse_opt_ref_sorting: always use with NONEG flag\n\nNicely written.  Thanks.\n\n>  builtin/branch.c            |  3 +--\n>  builtin/checkout.c          |  2 +-\n>  builtin/commit.c            |  6 +++---\n>  builtin/fetch-pack.c        |  2 +-\n>  builtin/for-each-ref.c      |  3 +--\n>  builtin/log.c               | 18 +++++++++---------\n>  builtin/ls-files.c          |  2 +-\n>  builtin/ls-remote.c         |  3 +--\n>  builtin/submodule--helper.c |  2 +-\n>  builtin/tag.c               |  3 +--\n>  builtin/update-index.c      |  5 ++---\n>  dir.c                       |  3 +--\n>  dir.h                       |  2 +-\n>  fetch-pack.c                |  3 +--\n>  fetch-pack.h                |  3 +--\n>  parse-options.c             |  5 ++---\n>  pretty.c                    | 12 +++++-------\n>  ref-filter.c                |  9 +++++++--\n>  ref-filter.h                |  5 +++++\n>  revision.c                  | 12 ++++++------\n>  t/helper/test-date.c        |  4 ++--\n>  transport.c                 | 10 ++++------\n>  unpack-trees.c              |  9 +++------\n>  23 files changed, 60 insertions(+), 66 deletions(-)\n>\n> -Peff\n"},{"id":"372003","messageId":"CAN0heSrubOaXRbg=NBi0rCuzthUcOEU3HmUKGg+=t+N0CCJcbA@mail.gmail.com","threadId":"50786","inReplyTo":"20190320081650.GM10403@sigill.intra.peff.net","subject":"Re: [PATCH 13/13] parse_opt_ref_sorting: always use with NONEG flag","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2019-03-20T12:22:22Z","receivedAt":"2019-03-20T12:22:36Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 20 Mar 2019 at 09:17, Jeff King <peff@peff.net> wrote:\n>   - since this was cut-and-pasted to four different spots, let's define\n>     a single OPT_REF_SORT() macro that we can use everywhere\n\nIndeed, all four are identical. And FWIW I failed to find a fifth caller\nanywhere (I looked for \"OPT_CALLBACK.*sort\").\n\n> -               OPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n> -                            N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n> +               OPT_REF_SORT(sorting_tail),\n\n> +#define OPT_REF_SORT(var) \\\n> +       OPT_CALLBACK_F(0, \"sort\", (var), \\\n> +                      N_(\"key\"), N_(\"field name to sort\"), \\\n> +                      PARSE_OPT_NONEG, parse_opt_ref_sorting)\n\nThis one is not identical though. ;-) You drop the \"on\". I trust you to\nknow which of these is (more) correct, but I was a bit surprised to see\n\"on\" disappear without any mention. Mistake, or intended?\n\nOther than that surprise ending, the whole series was a nice read.\n\nMartin\n"},{"id":"372021","messageId":"20190320202215.GA17382@sigill.intra.peff.net","threadId":"50786","inReplyTo":"CAN0heSrubOaXRbg=NBi0rCuzthUcOEU3HmUKGg+=t+N0CCJcbA@mail.gmail.com","subject":"Re: [PATCH 13/13] parse_opt_ref_sorting: always use with NONEG flag","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-20T20:22:15Z","receivedAt":"2019-03-20T20:22:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2019 at 01:22:22PM +0100, Martin Ågren wrote:\n\n> > +#define OPT_REF_SORT(var) \\\n> > +       OPT_CALLBACK_F(0, \"sort\", (var), \\\n> > +                      N_(\"key\"), N_(\"field name to sort\"), \\\n> > +                      PARSE_OPT_NONEG, parse_opt_ref_sorting)\n> \n> This one is not identical though. ;-) You drop the \"on\". I trust you to\n> know which of these is (more) correct, but I was a bit surprised to see\n> \"on\" disappear without any mention. Mistake, or intended?\n\nMistake. I retyped when I should have cut-and-pasted.\n\nAmusingly, I even carefully checked that the original four were all\nidentical, but didn't think to check against my new one. Revised patch\nbelow.\n\n> Other than that surprise ending, the whole series was a nice read.\n\nI'm the M. Night Shyamalan of Git development.\n\nThanks for reading. :)\n\n-- >8 --\nSubject: [PATCH] parse_opt_ref_sorting: always use with NONEG flag\n\nThe \"--sort\" parameter of for-each-ref, etc, does not handle negation,\nand instead returns an error to the parse-options code. But neither\npiece of code prints anything for the user, which may leave them\nconfused:\n\n  $ git for-each-ref --no-sort\n  $ echo $?\n  129\n\nAs the comment in the callback function notes, this probably should\nclear the list, which would make it consistent with other list-like\noptions (i.e., anything that uses OPT_STRING_LIST currently).\nUnfortunately that's a bit tricky due to the way the ref-filter code\nworks. But in the meantime, let's at least make the error a little less\nconfusing:\n\n  - switch to using PARSE_OPT_NONEG in the option definition, which will\n    cause the options code to produce a useful message\n\n  - since this was cut-and-pasted to four different spots, let's define\n    a single OPT_REF_SORT() macro that we can use everywhere\n\n  - the callback can use BUG_ON_OPT_NEG() to make sure the correct flags\n    are used (incidentally, this also satisfies -Wunused-parameters,\n    since we're now looking at \"unset\")\n\n  - expand the comment into a NEEDSWORK to make it clear that the\n    direction is right, but the details need to be worked out\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/branch.c       | 3 +--\n builtin/for-each-ref.c | 3 +--\n builtin/ls-remote.c    | 3 +--\n builtin/tag.c          | 3 +--\n ref-filter.c           | 9 +++++++--\n ref-filter.h           | 5 +++++\n 6 files changed, 16 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 4c83055730..d4359b33ac 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -644,8 +644,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \t\tOPT_MERGED(&filter, N_(\"print only branches that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only branches that are not merged\")),\n \t\tOPT_COLUMN(0, \"column\", &colopts, N_(\"list branches in columns\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only branches of the object\"), 0, parse_opt_object_name\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex e931be9ce4..465153e853 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -37,8 +37,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)\n \t\tOPT_INTEGER( 0 , \"count\", &maxcount, N_(\"show only <n> matched refs\")),\n \t\tOPT_STRING(  0 , \"format\", &format.format, N_(\"format\"), N_(\"format to use for the output\")),\n \t\tOPT__COLOR(&format.use_color, N_(\"respect format colors\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t    N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\tOPT_CALLBACK(0, \"points-at\", &filter.points_at,\n \t\t\t     N_(\"object\"), N_(\"print only refs which points at the given object\"),\n \t\t\t     parse_opt_object_name),\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 1d7f1f5ce2..6ef519514b 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -67,8 +67,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT(0, \"refs\", &flags, N_(\"do not show peeled tags\"), REF_NORMAL),\n \t\tOPT_BOOL(0, \"get-url\", &get_url,\n \t\t\t N_(\"take url.<base>.insteadOf into account\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\tOPT_SET_INT_F(0, \"exit-code\", &status,\n \t\t\t      N_(\"exit with exit code 2 if no matching refs are found\"),\n \t\t\t      2, PARSE_OPT_NOCOMPLETE),\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 02f6bd1279..ad97595fbf 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -412,8 +412,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \t\tOPT_WITHOUT(&filter.no_commit, N_(\"print only tags that don't contain the commit\")),\n \t\tOPT_MERGED(&filter, N_(\"print only tags that are merged\")),\n \t\tOPT_NO_MERGED(&filter, N_(\"print only tags that are not merged\")),\n-\t\tOPT_CALLBACK(0 , \"sort\", sorting_tail, N_(\"key\"),\n-\t\t\t     N_(\"field name to sort on\"), &parse_opt_ref_sorting),\n+\t\tOPT_REF_SORT(sorting_tail),\n \t\t{\n \t\t\tOPTION_CALLBACK, 0, \"points-at\", &filter.points_at, N_(\"object\"),\n \t\t\tN_(\"print only tags of the object\"), PARSE_OPT_LASTARG_DEFAULT,\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 3aca105307..8d11a94cbd 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -2337,8 +2337,13 @@ void parse_ref_sorting(struct ref_sorting **sorting_tail, const char *arg)\n \n int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)\n {\n-\tif (!arg) /* should --no-sort void the list ? */\n-\t\treturn -1;\n+\t/*\n+\t * NEEDSWORK: We should probably clear the list in this case, but we've\n+\t * already munged the global used_atoms list, which would need to be\n+\t * undone.\n+\t */\n+\tBUG_ON_OPT_NEG(unset);\n+\n \tparse_ref_sorting(opt->value, arg);\n \treturn 0;\n }\ndiff --git a/ref-filter.h b/ref-filter.h\nindex 85c8ebc3b9..f1dcff4c6e 100644\n--- a/ref-filter.h\n+++ b/ref-filter.h\n@@ -96,6 +96,11 @@ struct ref_format {\n #define OPT_MERGED(f, h) _OPT_MERGED_NO_MERGED(\"merged\", f, h)\n #define OPT_NO_MERGED(f, h) _OPT_MERGED_NO_MERGED(\"no-merged\", f, h)\n \n+#define OPT_REF_SORT(var) \\\n+\tOPT_CALLBACK_F(0, \"sort\", (var), \\\n+\t\t       N_(\"key\"), N_(\"field name to sort on\"), \\\n+\t\t       PARSE_OPT_NONEG, parse_opt_ref_sorting)\n+\n /*\n  * API for filtering a set of refs. Based on the type of refs the user\n  * has requested, we iterate through those refs and apply filters\n-- \n2.21.0.701.g4401309e11\n\n"},{"id":"372075","messageId":"87woksd322.fsf@evledraar.gmail.com","threadId":"50786","inReplyTo":"20190320081258.GA5621@sigill.intra.peff.net","subject":"Re: [PATCH 0/13] more unused parameter cleanups","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-03-21T08:50:13Z","receivedAt":"2019-03-21T08:50:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Mar 20 2019, Jeff King wrote:\n\n> Here's another round of -Wunused-parameter cleanups. Previous rounds\n> were at [1] and [2]. As before, these are mostly just removals, so it's\n> easy to see there's no behavior change (there are a couple of cleanups\n> that ride along, though, so watch for those).\n>\n> There are two minor conflicts when merging with pu:\n>\n>   - jt/fetch-cdn-offload tweaked the \"pack_lockfile\" parameter to\n>     fetch_pack(). A few other parameters are dropped in this series.\n>     The textual resolution is pretty straightforward.\n>\n>   - ps/stash-in-c (et al) added a new call to report_path_error() in\n>     builtin/stash.c, which here loses its redundant \"prefix\" parameter.\n>     There's no textual conflict, but the new call needs to drop its\n>     final NULL parameter in order to compile.\n\nLGTM from skimming it, FWIW this is now what we need to compile cleanly\nwith -Wextra:\n\n    make DEVELOPER=1 DEVOPTS=\"extra-all\" CFLAGS=\"-Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-empty-body\" all\n\nFor some such as -Wempty-body we'd really need to contort ourselves to\nget it passing anywhere near cleanly (all of those have existing \"/*\nthis is intentional! */\" comments).\n\nI wonder if for the rest of these it's worth re-picking up this old\nsuggestions of yours about #pragma:\nhttps://public-inbox.org/git/20170126143252.ne533mcv3n2ksbai@sigill.intra.peff.net/\n\nI.e. for us to define our own macro for these cases & use it.\n\nhttps://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html\nhttps://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas\n\nI was looking into this for SunCC the other day, it has various new\nwarnings that are useful that neither gcc nor clang (or anything else\nI've tried) has, but also has some stupidities due to faulty code\nanalysis, luckily those can be disabled:\n\nhttps://docs.oracle.com/cd/E19205-01/819-5265/bjaby/index.html\n\nThis would allow me to compile there with -Werror.\n\nIt would mean quite some macro verbosity in some existing code, maybe\nit's not worth it...\n"},{"id":"372083","messageId":"20190321094414.GA2894@sigill.intra.peff.net","threadId":"50786","inReplyTo":"87woksd322.fsf@evledraar.gmail.com","subject":"Re: [PATCH 0/13] more unused parameter cleanups","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-03-21T09:44:14Z","receivedAt":"2019-03-21T09:44:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Mar 21, 2019 at 09:50:13AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> LGTM from skimming it, FWIW this is now what we need to compile cleanly\n> with -Wextra:\n> \n>     make DEVELOPER=1 DEVOPTS=\"extra-all\" CFLAGS=\"-Wno-unused-parameter -Wno-missing-field-initializers -Wno-sign-compare -Wno-empty-body\" all\n> \n> For some such as -Wempty-body we'd really need to contort ourselves to\n> get it passing anywhere near cleanly (all of those have existing \"/*\n> this is intentional! */\" comments).\n\nI think we could probably define a NOOP_BODY macro or function and use\nthat instead. But it may not be worth the trouble. I'd have to see how\npainful that would be, and whether it might find any cases that actually\nlook like real bugs.\n\nFor -Wunused-parameter I am working towards being able to actually\nenable that everywhere. It is not _too_ bad to annotate the instances\nwhich must be there, and my digging with it has uncovered several real\nbugs. Right now I'm in the \"drop useless parameters\" phase, which I\nexpect will take one or two more rounds.\n\nThen I'll start on the \"annotate unused ones we must keep\" series, which\nculminates in actually flipping on the switch with DEVELOPER (or rather,\nstopping flipping it off).\n\nYou can see my progress on the jk/unused-4-mark branch of\nhttps://github.com/peff/git (I think the contents are good, but the\ncommit messages and organization need some cleanup).\n\n> I wonder if for the rest of these it's worth re-picking up this old\n> suggestions of yours about #pragma:\n> https://public-inbox.org/git/20170126143252.ne533mcv3n2ksbai@sigill.intra.peff.net/\n> \n> I.e. for us to define our own macro for these cases & use it.\n\nThe push/pop ones may be of use (which both clang and gcc seem to\nsupport), since that would let us localize the effects. I think in many\ncases there's usually a more readable solution, though (e.g., you'd want\nto annotate specific parameters as unused with single word, not a 3-line\npush-diag/declare-param/pop-diag mess).\n\n-Peff\n"}]}