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

[PATCH v3 14/27] help: refactor to not use globals for reading config

From
Patrick Steinhardt <ps@pks.im>
Date
Nov 20, 2024, 13:39 UTC
Message-ID
<20241120-b4-pks-leak-fixes-pt10-v3-14-d67f08f45c74@pks.im>
In-Reply-To
<20241120-b4-pks-leak-fixes-pt10-v3-0-d67f08f45c74@pks.im>

We're reading the "help.autocorrect" and "alias.*" configuration into global variables, which makes it hard to manage their lifetime correctly. Refactor the code to use a struct instead.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 help.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)
diff --git a/help.c b/help.c
index 4ad4ebdd2cfddb3191bb4f6d593a55d3c5d3ed14..8794f81db9bbf457b7cabca59222299922f3cc7f 100644
--- a/help.c
+++ b/help.c
@@ -546,8 +546,10 @@ int is_in_cmdlist(struct cmdnames *c, const char *s)
 	return 0;
 }
 
-static int autocorrect;
-static struct cmdnames aliases;
+struct help_unknown_cmd_config {
+	int autocorrect;
+	struct cmdnames aliases;
+};
 
 #define AUTOCORRECT_PROMPT (-3)
 #define AUTOCORRECT_NEVER (-2)
@@ -555,28 +557,29 @@ static struct cmdnames aliases;
 
 static int git_unknown_cmd_config(const char *var, const char *value,
 				  const struct config_context *ctx,
-				  void *cb UNUSED)
+				  void *cb)
 {
+	struct help_unknown_cmd_config *cfg = cb;
 	const char *p;
 
 	if (!strcmp(var, "help.autocorrect")) {
 		if (!value)
 			return config_error_nonbool(var);
 		if (!strcmp(value, "never")) {
-			autocorrect = AUTOCORRECT_NEVER;
+			cfg->autocorrect = AUTOCORRECT_NEVER;
 		} else if (!strcmp(value, "immediate")) {
-			autocorrect = AUTOCORRECT_IMMEDIATELY;
+			cfg->autocorrect = AUTOCORRECT_IMMEDIATELY;
 		} else if (!strcmp(value, "prompt")) {
-			autocorrect = AUTOCORRECT_PROMPT;
+			cfg->autocorrect = AUTOCORRECT_PROMPT;
 		} else {
 			int v = git_config_int(var, value, ctx->kvi);
-			autocorrect = (v < 0)
+			cfg->autocorrect = (v < 0)
 				? AUTOCORRECT_IMMEDIATELY : v;
 		}
 	}
 	/* Also use aliases for command lookup */
 	if (skip_prefix(var, "alias.", &p))
-		add_cmdname(&aliases, p, strlen(p));
+		add_cmdname(&cfg->aliases, p, strlen(p));
 
 	return 0;
 }
@@ -611,30 +614,28 @@ static const char bad_interpreter_advice[] =
 
 const char *help_unknown_cmd(const char *cmd)
 {
+	struct help_unknown_cmd_config cfg = { 0 };
 	int i, n, best_similarity = 0;
-	struct cmdnames main_cmds, other_cmds;
+	struct cmdnames main_cmds = { 0 };
+	struct cmdnames other_cmds = { 0 };
 	struct cmdname_help *common_cmds;
 
-	memset(&main_cmds, 0, sizeof(main_cmds));
-	memset(&other_cmds, 0, sizeof(other_cmds));
-	memset(&aliases, 0, sizeof(aliases));
-
-	read_early_config(the_repository, git_unknown_cmd_config, NULL);
+	read_early_config(the_repository, git_unknown_cmd_config, &cfg);
 
 	/*
 	 * Disable autocorrection prompt in a non-interactive session
 	 */
-	if ((autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
-		autocorrect = AUTOCORRECT_NEVER;
+	if ((cfg.autocorrect == AUTOCORRECT_PROMPT) && (!isatty(0) || !isatty(2)))
+		cfg.autocorrect = AUTOCORRECT_NEVER;
 
-	if (autocorrect == AUTOCORRECT_NEVER) {
+	if (cfg.autocorrect == AUTOCORRECT_NEVER) {
 		fprintf_ln(stderr, _("git: '%s' is not a git command. See 'git --help'."), cmd);
 		exit(1);
 	}
 
 	load_command_list("git-", &main_cmds, &other_cmds);
 
-	add_cmd_list(&main_cmds, &aliases);
+	add_cmd_list(&main_cmds, &cfg.aliases);
 	add_cmd_list(&main_cmds, &other_cmds);
 	QSORT(main_cmds.names, main_cmds.cnt, cmdname_compare);
 	uniq(&main_cmds);
@@ -693,7 +694,7 @@ const char *help_unknown_cmd(const char *cmd)
 		     n++)
 			; /* still counting */
 	}
-	if (autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {
+	if (cfg.autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) {
 		const char *assumed = main_cmds.names[0]->name;
 		main_cmds.names[0] = NULL;
 		cmdnames_release(&main_cmds);
@@ -701,12 +702,12 @@ const char *help_unknown_cmd(const char *cmd)
 			   _("WARNING: You called a Git command named '%s', "
 			     "which does not exist."),
 			   cmd);
-		if (autocorrect == AUTOCORRECT_IMMEDIATELY)
+		if (cfg.autocorrect == AUTOCORRECT_IMMEDIATELY)
 			fprintf_ln(stderr,
 				   _("Continuing under the assumption that "
 				     "you meant '%s'."),
 				   assumed);
-		else if (autocorrect == AUTOCORRECT_PROMPT) {
+		else if (cfg.autocorrect == AUTOCORRECT_PROMPT) {
 			char *answer;
 			struct strbuf msg = STRBUF_INIT;
 			strbuf_addf(&msg, _("Run '%s' instead [y/N]? "), assumed);
@@ -719,8 +720,8 @@ const char *help_unknown_cmd(const char *cmd)
 			fprintf_ln(stderr,
 				   _("Continuing in %0.1f seconds, "
 				     "assuming that you meant '%s'."),
-				   (float)autocorrect/10.0, assumed);
-			sleep_millisec(autocorrect * 100);
+				   (float)cfg.autocorrect/10.0, assumed);
+			sleep_millisec(cfg.autocorrect * 100);
 		}
 		return assumed;
 	}
-- 
2.47.0.274.g962d0b743d.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 19 of 33 in “Memory leak fixes (pt.10, final)”
  1. 00/27 Memory leak fixes (pt.10, final)Patrick Steinhardt, Nov 20, 2024
  2. 01/27 builtin/blame: fix leaking blame entries with `--incremental`Patrick Steinhardt, Nov 20, 2024
  3. 02/27 bisect: fix leaking good/bad terms when reading multipe timesPatrick Steinhardt, Nov 20, 2024
  4. 04/27 bisect: fix leaking `current_bad_oid`Patrick Steinhardt, Nov 20, 2024
  5. 03/27 bisect: fix leaking string in `handle_bad_merge_base()`Patrick Steinhardt, Nov 20, 2024
  6. 05/27 bisect: fix multiple leaks in `bisect_next_all()`Patrick Steinhardt, Nov 20, 2024
  7. 06/27 bisect: fix leaking commit list items in `check_merge_base()`Patrick Steinhardt, Nov 20, 2024
  8. 07/27 bisect: fix various cases where we leak commit list itemsPatrick Steinhardt, Nov 20, 2024
  9. Jeff KingNov 25, 2024
  10. Patrick SteinhardtNov 25, 2024
  11. Jeff KingNov 25, 2024
  12. Patrick SteinhardtNov 25, 2024
  13. 08/27 line-log: fix leak when rewriting commit parentsPatrick Steinhardt, Nov 20, 2024
  14. 09/27 strvec: introduce new `strvec_splice()` functionPatrick Steinhardt, Nov 20, 2024
  15. 10/27 git: refactor alias handling to use a `struct strvec`Patrick Steinhardt, Nov 20, 2024
  16. 11/27 git: refactor builtin handling to use a `struct strvec`Patrick Steinhardt, Nov 20, 2024
  17. 12/27 split-index: fix memory leak in `move_cache_to_base_index()`Patrick Steinhardt, Nov 20, 2024
  18. 13/27 builtin/sparse-checkout: fix leaking sanitized patternsPatrick Steinhardt, Nov 20, 2024
  19. 14/27 help: refactor to not use globals for reading configPatrick Steinhardt, Nov 20, 2024
  20. 15/27 help: fix leaking `struct cmdnames`Patrick Steinhardt, Nov 20, 2024
  21. 16/27 help: fix leaking return value from `help_unknown_cmd()`Patrick Steinhardt, Nov 20, 2024
  22. 17/27 builtin/help: fix leaks in `check_git_cmd()`Patrick Steinhardt, Nov 20, 2024
  23. 18/27 builtin/init-db: fix leaking directory pathsPatrick Steinhardt, Nov 20, 2024
  24. 20/27 t/helper: fix leaking commit graph in "read-graph" subcommandPatrick Steinhardt, Nov 20, 2024
  25. 19/27 builtin/branch: fix leaking sorting optionsPatrick Steinhardt, Nov 20, 2024
  26. 21/27 global: drop `UNLEAK()` annotationPatrick Steinhardt, Nov 20, 2024
  27. 22/27 git-compat-util: drop now-unused `UNLEAK()` macroPatrick Steinhardt, Nov 20, 2024
  28. 23/27 t5601: work around leak sanitizer issuePatrick Steinhardt, Nov 20, 2024
  29. 24/27 t: mark some tests as leak freePatrick Steinhardt, Nov 20, 2024
  30. 25/27 t: remove unneeded !SANITIZE_LEAK prerequisitesPatrick Steinhardt, Nov 20, 2024
  31. 26/27 test-lib: unconditionally enable leak checkingPatrick Steinhardt, Nov 20, 2024
  32. 27/27 t: remove TEST_PASSES_SANITIZE_LEAK annotationsPatrick Steinhardt, Nov 20, 2024
  33. Toon ClaesNov 21, 2024

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.