From 94aa96cd590c7da5103c61795e34de4d3ae2849e Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 20 Nov 2024 14:39:43 +0100 Subject: help: refactor to not use globals for reading config 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 Signed-off-by: Junio C Hamano --- help.c | 47 ++++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 23 deletions(-) (limited to 'help.c') diff --git a/help.c b/help.c index 4ad4ebdd2c..8794f81db9 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; } -- cgit v1.2.3 From 889c597961cb8dfc0255f520a270aafe125b9869 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 20 Nov 2024 14:39:44 +0100 Subject: help: fix leaking `struct cmdnames` We're populating multiple `struct cmdnames`, but don't ever free them. Plug this memory leak. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- help.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'help.c') diff --git a/help.c b/help.c index 8794f81db9..8b56cd6e25 100644 --- a/help.c +++ b/help.c @@ -723,6 +723,10 @@ const char *help_unknown_cmd(const char *cmd) (float)cfg.autocorrect/10.0, assumed); sleep_millisec(cfg.autocorrect * 100); } + + cmdnames_release(&cfg.aliases); + cmdnames_release(&main_cmds); + cmdnames_release(&other_cmds); return assumed; } -- cgit v1.2.3 From 7720dbe99b303b3d658898587e02d7cf224a93c3 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 20 Nov 2024 14:39:45 +0100 Subject: help: fix leaking return value from `help_unknown_cmd()` While `help_unknown_cmd()` would usually die on an unknown command, it instead returns an autocorrected command when "help.autocorrect" is set. But while the function is declared to return a string constant, it actually returns an allocated string in that case. Callers thus aren't aware that they have to free the string, leading to a memory leak. Fix the function return type to be non-constant and free the returned value at its only callsite. Note that we cannot simply take ownership of `main_cmds.names[0]->name` and then eventually free it. This is because the `struct cmdname` is using a flex array to allocate the name, so the name pointer points into the middle of the structure and thus cannot be freed. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano --- git.c | 4 +++- help.c | 7 +++---- help.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) (limited to 'help.c') diff --git a/git.c b/git.c index 159dd45b08..46b3c740c5 100644 --- a/git.c +++ b/git.c @@ -961,7 +961,9 @@ int cmd_main(int argc, const char **argv) exit(1); } if (!done_help) { - strvec_replace(&args, 0, help_unknown_cmd(cmd)); + char *assumed = help_unknown_cmd(cmd); + strvec_replace(&args, 0, assumed); + free(assumed); cmd = args.v[0]; done_help = 1; } else { diff --git a/help.c b/help.c index 8b56cd6e25..8a830ba35c 100644 --- a/help.c +++ b/help.c @@ -612,7 +612,7 @@ static const char bad_interpreter_advice[] = N_("'%s' appears to be a git command, but we were not\n" "able to execute it. Maybe git-%s is broken?"); -const char *help_unknown_cmd(const char *cmd) +char *help_unknown_cmd(const char *cmd) { struct help_unknown_cmd_config cfg = { 0 }; int i, n, best_similarity = 0; @@ -695,9 +695,8 @@ const char *help_unknown_cmd(const char *cmd) ; /* still counting */ } 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); + char *assumed = xstrdup(main_cmds.names[0]->name); + fprintf_ln(stderr, _("WARNING: You called a Git command named '%s', " "which does not exist."), diff --git a/help.h b/help.h index e716ee27ea..67207b3073 100644 --- a/help.h +++ b/help.h @@ -32,7 +32,7 @@ void list_all_other_cmds(struct string_list *list); void list_cmds_by_category(struct string_list *list, const char *category); void list_cmds_by_config(struct string_list *list); -const char *help_unknown_cmd(const char *cmd); +char *help_unknown_cmd(const char *cmd); void load_command_list(const char *prefix, struct cmdnames *main_cmds, struct cmdnames *other_cmds); -- cgit v1.2.3