git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/4] commit: avoid writing to global in option callback

From
Jeff King <peff@peff.net>
Date
Oct 6, 2022, 13:11 UTC
Message-ID
<Yz7UA2cdxVPellOe@coredump.intra.peff.net>
In-Reply-To
<Yz7Tjy7Rh8cXVxYQ@coredump.intra.peff.net>

The callback function for --trailer writes directly to the global trailer_args and ignores opt->value completely. This is OK, since that's where we expect to find the value. But it does mean the option declaration isn't as clear. E.g., we have:

    OPT_BOOL(0, "reset-author", &renew_authorship, ...),
    OPT_CALLBACK_F(0, "trailer", NULL, ..., opt_pass_trailer)

In the first one we can see where the result will be stored, but in the second, we get only NULL, and you have to go read the callback.

Let's pass &trailer_args, and use it in the callback. As a bonus, this silences a -Wunused-parameter warning.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/commit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index fcf9c85947..d9de4ef008 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -139,7 +139,7 @@ static int opt_pass_trailer(const struct option *opt, const char *arg, int unset
 {
 	BUG_ON_OPT_NEG(unset);
 
-	strvec_pushl(&trailer_args, "--trailer", arg, NULL);
+	strvec_pushl(opt->value, "--trailer", arg, NULL);
 	return 0;
 }
 
@@ -1633,7 +1633,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "fixup", &fixup_message, N_("[(amend|reword):]commit"), N_("use autosquash formatted message to fixup or amend/reword specified commit")),
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
-		OPT_CALLBACK_F(0, "trailer", NULL, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, opt_pass_trailer),
+		OPT_CALLBACK_F(0, "trailer", &trailer_args, N_("trailer"), N_("add custom trailer(s)"), PARSE_OPT_NONEG, opt_pass_trailer),
 		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
-- 
2.38.0.661.g581b1c9920
Previous: Jeff KingNext: Jeff King
Message 4 of 11 in “a few small unused-parameter fixes”
  1. 0/4 a few small unused-parameter fixesJeff King, Oct 6, 2022
  2. 1/4 test-submodule: inline resolve_relative_url() functionJeff King, Oct 6, 2022
  3. 2/4 multi-pack-index: avoid writing to global in option callbackJeff King, Oct 6, 2022
  4. 3/4 commit: avoid writing to global in option callbackJeff King, Oct 6, 2022
  5. 4/4 attr: convert DEBUG_ATTR to use trace APIJeff King, Oct 6, 2022
  6. attr: drop DEBUG_ATTR codeJeff King, Oct 6, 2022
  7. Junio C HamanoOct 6, 2022
  8. Derrick StoleeOct 6, 2022
  9. Junio C HamanoOct 6, 2022
  10. Jeff KingOct 11, 2022
  11. Jeff KingOct 11, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.