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

[PATCH v2 07/10] merge: do not pass unused opt->value parameter

From
Jeff King <peff@peff.net>
Date
Aug 31, 2023, 21:21 UTC
Message-ID
<20230831212146.GG949469@coredump.intra.peff.net>
In-Reply-To
<20230831211637.GA949188@coredump.intra.peff.net>

The option_parse_strategy() callback does not look at opt->value; instead it calls append_strategy(), which manipulates the global use_strategies array directly. But the OPT_CALLBACK declaration assigns "&use_strategies" to opt->value.

One could argue this is good, as it tells the reader what we generally expect the callback to do. But it is also bad, because it can mislead you into thinking that swapping out "&use_strategies" there might have any effect. Let's switch it to pass NULL (which is what every other "does not bother to look at opt->value" callback does). If you want to know what the callback does, it's easy to read the function itself.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/merge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 0436986dab..545da0c8a1 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -264,7 +264,7 @@ static struct option builtin_merge_options[] = {
 	OPT_RERERE_AUTOUPDATE(&allow_rerere_auto),
 	OPT_BOOL(0, "verify-signatures", &verify_signatures,
 		N_("verify that the named commit has a valid GPG signature")),
-	OPT_CALLBACK('s', "strategy", &use_strategies, N_("strategy"),
+	OPT_CALLBACK('s', "strategy", NULL, N_("strategy"),
 		N_("merge strategy to use"), option_parse_strategy),
 	OPT_STRVEC('X', "strategy-option", &xopts, N_("option=value"),
 		N_("option for selected merge strategy")),
-- 
2.42.0.561.gaa987ecc69
Previous: René ScharfeNext: Jeff King
Message 41 of 47 in “more unused parameters in parseopt callbacks”
  1. 0/8 more unused parameters in parseopt callbacksJeff King, Aug 31, 2023
  2. 1/8 merge: make xopts a strvecJeff King, Aug 31, 2023
  3. Jeff KingAug 31, 2023
  4. Phillip WoodAug 31, 2023
  5. Junio C HamanoAug 31, 2023
  6. Taylor BlauAug 31, 2023
  7. 2/8 merge: simplify parsing of "-n" optionJeff King, Aug 31, 2023
  8. Junio C HamanoAug 31, 2023
  9. 3/8 parse-options: prefer opt->value to globals in callbacksJeff King, Aug 31, 2023
  10. Junio C HamanoAug 31, 2023
  11. 4/8 parse-options: mark unused "opt" parameter in callbacksJeff King, Aug 31, 2023
  12. Junio C HamanoAug 31, 2023
  13. Jeff KingAug 31, 2023
  14. Jeff KingAug 31, 2023
  15. 5/8 merge: do not pass unused opt->value parameterJeff King, Aug 31, 2023
  16. Junio C HamanoAug 31, 2023
  17. 6/8 parse-options: add more BUG_ON() annotationsJeff King, Aug 31, 2023
  18. Junio C HamanoAug 31, 2023
  19. 7/8 interpret-trailers: mark unused "unset" parameters in option callbacksJeff King, Aug 31, 2023
  20. Junio C HamanoAug 31, 2023
  21. Jeff KingAug 31, 2023
  22. 8/8 parse-options: mark unused parameters in noop callbackJeff King, Aug 31, 2023
  23. Junio C HamanoAug 31, 2023
  24. 0/10 more unused parameters in parseopt callbacksJeff King, Aug 31, 2023
  25. 01/10 merge: make xopts a strvecJeff King, Aug 31, 2023
  26. 02/10 merge: simplify parsing of "-n" optionJeff King, Aug 31, 2023
  27. René ScharfeSep 2, 2023
  28. Jeff KingSep 5, 2023
  29. 03/10 format-patch: use OPT_STRING_LIST for to/cc optionsJeff King, Aug 31, 2023
  30. 04/10 checkout-index: delay automatic setting of to_tempfileJeff King, Aug 31, 2023
  31. Junio C HamanoAug 31, 2023
  32. René ScharfeSep 2, 2023
  33. 04/10 checkout-index: delay automatic setting of to_tempfileJeff King, Sep 5, 2023
  34. 05/10 parse-options: prefer opt->value to globals in callbacksJeff King, Aug 31, 2023
  35. René ScharfeSep 2, 2023
  36. Jeff KingSep 5, 2023
  37. 06/10 parse-options: mark unused "opt" parameter in callbacksJeff King, Aug 31, 2023
  38. René ScharfeSep 2, 2023
  39. Jeff KingSep 5, 2023
  40. René ScharfeSep 19, 2023
  41. 07/10 merge: do not pass unused opt->value parameterJeff King, Aug 31, 2023
  42. 08/10 parse-options: add more BUG_ON() annotationsJeff King, Aug 31, 2023
  43. 09/10 interpret-trailers: mark unused "unset" parameters in option callbacksJeff King, Aug 31, 2023
  44. 10/10 parse-options: mark unused parameters in noop callbackJeff King, Aug 31, 2023
  45. René ScharfeSep 2, 2023
  46. Jeff KingSep 5, 2023
  47. René ScharfeSep 7, 2023

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.