[v2,0/10] more unused parameters in parseopt callbacks

Message

Jeff King Aug. 31, 2023, 9:16 p.m. UTC

On Thu, Aug 31, 2023 at 03:09:35AM -0400, Jeff King wrote:

> Here are some more patches silencing -Wunused-parameter warnings. I've
> prepared them on top of the patches queued in jk/unused-post-2.42, but
> they should apply equally well directly on 'master'.

And here's a re-roll based on feedback from Junio:

  [01/10]: merge: make xopts a strvec
  [02/10]: merge: simplify parsing of "-n" option
  [03/10]: format-patch: use OPT_STRING_LIST for to/cc options
  [04/10]: checkout-index: delay automatic setting of to_tempfile
  [05/10]: parse-options: prefer opt->value to globals in callbacks
  [06/10]: parse-options: mark unused "opt" parameter in callbacks
  [07/10]: merge: do not pass unused opt->value parameter
  [08/10]: parse-options: add more BUG_ON() annotations
  [09/10]: interpret-trailers: mark unused "unset" parameters in option callbacks
  [10/10]: parse-options: mark unused parameters in noop callback

Range-diff is below, but the most interesting changes are in the two new
patches (3 and 4):

  - patch 3 simplifies away a few string-list callbacks (which can then
    be omitted from patch 6, the "mark unused opt" one).

  - patch 4 fixes some minor bugs with "checkout-index --stage". In
    doing so, it's now eligible for cleanup in patch 5 ("prefer
    opt->value") and its annotation dropped from patch 6

  - patch 5 ("prefer opt->value") converts the missed refmap callback,
    and so its annotation is gone from patch 6 now

  - a few extra clarifying comments

 1:  8750d92016 !  1:  82940b6936 merge: make xopts a strvec
    @@ Commit message
     
         As a bonus, this means that "--no-strategy-option", which was previously
         a silent noop, now does something useful: like other list-like options,
    -    it will clear the list of -X options seen so far.
    +    it will clear the list of -X options seen so far. This matches the
    +    behavior of revert/cherry-pick, which made the same change in fb60b9f37f
    +    (sequencer: use struct strvec to store merge strategy options,
    +    2023-04-10).
     
         Signed-off-by: Jeff King <[e-mail]>
     
 2:  ffdae69bd6 =  2:  c98b87feb9 merge: simplify parsing of "-n" option
 -:  ---------- >  3:  615fef8f90 format-patch: use OPT_STRING_LIST for to/cc options
 -:  ---------- >  4:  66cf7bcf08 checkout-index: delay automatic setting of to_tempfile
 3:  f00c78cc49 !  5:  a7c2bfecba parse-options: prefer opt->value to globals in callbacks
    @@ Commit message
     
         Signed-off-by: Jeff King <[e-mail]>
     
    + ## builtin/checkout-index.c ##
    +@@ builtin/checkout-index.c: static const char * const builtin_checkout_index_usage[] = {
    + static int option_parse_stage(const struct option *opt,
    + 			      const char *arg, int unset)
    + {
    ++	int *stage = opt->value;
    ++
    + 	BUG_ON_OPT_NEG(unset);
    + 
    + 	if (!strcmp(arg, "all")) {
    +-		checkout_stage = CHECKOUT_ALL;
    ++		*stage = CHECKOUT_ALL;
    + 	} else {
    + 		int ch = arg[0];
    + 		if ('1' <= ch && ch <= '3')
    +-			checkout_stage = arg[0] - '0';
    ++			*stage = arg[0] - '0';
    + 		else
    + 			die(_("stage should be between 1 and 3 or all"));
    + 	}
    +@@ builtin/checkout-index.c: int cmd_checkout_index(int argc, const char **argv, const char *prefix)
    + 			N_("write the content to temporary files")),
    + 		OPT_STRING(0, "prefix", &state.base_dir, N_("string"),
    + 			N_("when creating files, prepend <string>")),
    +-		OPT_CALLBACK_F(0, "stage", NULL, "(1|2|3|all)",
    ++		OPT_CALLBACK_F(0, "stage", &checkout_stage, "(1|2|3|all)",
    + 			N_("copy out the files from named stage"),
    + 			PARSE_OPT_NONEG, option_parse_stage),
    + 		OPT_END()
    +
      ## builtin/describe.c ##
     @@ builtin/describe.c: static void describe(const char *arg, int last_one)
      static int option_parse_exact_match(const struct option *opt, const char *arg,
    @@ builtin/fast-export.c: static int parse_opt_tag_of_filtered_mode(const struct op
      			return error("Unknown reencoding mode: %s", arg);
      	}
     
    + ## builtin/fetch.c ##
    +@@ builtin/fetch.c: static int parse_refmap_arg(const struct option *opt, const char *arg, int unset
    + 	 * "git fetch --refmap='' origin foo"
    + 	 * can be used to tell the command not to store anywhere
    + 	 */
    +-	refspec_append(&refmap, arg);
    ++	refspec_append(opt->value, arg);
    + 
    + 	return 0;
    + }
    +@@ builtin/fetch.c: int cmd_fetch(int argc, const char **argv, const char *prefix)
    + 			   PARSE_OPT_HIDDEN, option_fetch_parse_recurse_submodules),
    + 		OPT_BOOL(0, "update-shallow", &update_shallow,
    + 			 N_("accept refs that update .git/shallow")),
    +-		OPT_CALLBACK_F(0, "refmap", NULL, N_("refmap"),
    ++		OPT_CALLBACK_F(0, "refmap", &refmap, N_("refmap"),
    + 			       N_("specify fetch refmap"), PARSE_OPT_NONEG, parse_refmap_arg),
    + 		OPT_STRING_LIST('o', "server-option", &server_options, N_("server-specific"), N_("option to transmit")),
    + 		OPT_IPVERSION(&family),
    +
      ## builtin/interpret-trailers.c ##
     @@ builtin/interpret-trailers.c: static enum trailer_if_missing if_missing;
      static int option_parse_where(const struct option *opt,
 4:  19621fc01c !  6:  bd915fe4de parse-options: mark unused "opt" parameter in callbacks
    @@ Commit message
     
         Signed-off-by: Jeff King <[e-mail]>
     
    - ## builtin/checkout-index.c ##
    -@@ builtin/checkout-index.c: static const char * const builtin_checkout_index_usage[] = {
    - 	NULL
    - };
    - 
    --static int option_parse_stage(const struct option *opt,
    -+static int option_parse_stage(const struct option *opt UNUSED,
    - 			      const char *arg, int unset)
    - {
    - 	BUG_ON_OPT_NEG(unset);
    -
    - ## builtin/fetch.c ##
    -@@ builtin/fetch.c: static int git_fetch_config(const char *k, const char *v,
    - 	return git_default_config(k, v, ctx, cb);
    - }
    - 
    --static int parse_refmap_arg(const struct option *opt, const char *arg, int unset)
    -+static int parse_refmap_arg(const struct option *opt UNUSED,
    -+			    const char *arg, int unset)
    - {
    - 	BUG_ON_OPT_NEG(unset);
    - 
    -
      ## builtin/gc.c ##
     @@ builtin/gc.c: static void initialize_task_config(int schedule)
      	strbuf_release(&config_name);
    @@ builtin/log.c: static int inline_callback(const struct option *opt, const char *
      {
      	if (unset) {
      		string_list_clear(&extra_hdr, 0);
    -@@ builtin/log.c: static int header_callback(const struct option *opt, const char *arg, int unset)
    - 	return 0;
    - }
    - 
    --static int to_callback(const struct option *opt, const char *arg, int unset)
    -+static int to_callback(const struct option *opt UNUSED, const char *arg,
    -+		       int unset)
    - {
    - 	if (unset)
    - 		string_list_clear(&extra_to, 0);
    -@@ builtin/log.c: static int to_callback(const struct option *opt, const char *arg, int unset)
    - 	return 0;
    - }
    - 
    --static int cc_callback(const struct option *opt, const char *arg, int unset)
    -+static int cc_callback(const struct option *opt UNUSED, const char *arg,
    -+		       int unset)
    - {
    - 	if (unset)
    - 		string_list_clear(&extra_cc, 0);
     
      ## builtin/merge.c ##
     @@ builtin/merge.c: static void append_strategy(struct strategy *s)
 5:  2d6a3b9e1b =  7:  1720ddebc1 merge: do not pass unused opt->value parameter
 6:  7669758cc7 =  8:  a93f155f21 parse-options: add more BUG_ON() annotations
 7:  b80020daec !  9:  529573e281 interpret-trailers: mark unused "unset" parameters in option callbacks
    @@ Commit message
         false). But none of these do.
     
         So the code is fine as-is. But we'll want to mark the unused "unset"
    -    parameters to quiet -Wunused-parameter.
    +    parameters to quiet -Wunused-parameter. I've also added a comment to
    +    make this rather subtle situation more explicit.
     
         Signed-off-by: Jeff King <[e-mail]>
     
    @@ builtin/interpret-trailers.c: static enum trailer_if_exists if_exists;
     -			      const char *arg, int unset)
     +			      const char *arg, int unset UNUSED)
      {
    ++	/* unset implies NULL arg, which is handled in our helper */
      	return trailer_set_where(opt->value, arg);
      }
      
      static int option_parse_if_exists(const struct option *opt,
     -				  const char *arg, int unset)
     +				  const char *arg, int unset UNUSED)
      {
    ++	/* unset implies NULL arg, which is handled in our helper */
      	return trailer_set_if_exists(opt->value, arg);
      }
      
      static int option_parse_if_missing(const struct option *opt,
     -				   const char *arg, int unset)
     +				   const char *arg, int unset UNUSED)
      {
    ++	/* unset implies NULL arg, which is handled in our helper */
      	return trailer_set_if_missing(opt->value, arg);
      }
    + 
 8:  f21961ed23 = 10:  943161eaf2 parse-options: mark unused parameters in noop callback