Re: [PATCH v2 3/4] help.c: plug leaks with(out) help.autocorrect
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:20
Tay Ray Chuan [off-list ref] writes:
When help.autocorrect is set, in an attempt to retain the memory to the string name in main_cmds, we unfortunately leaked the struct cmdname that held it. Fix this by creating a copy of the string name.
If you are updating help_unknown_cmd() so that the caller can free (and is responsible for freeing if it wants to avoid leaks) the piece of memory it returns, that change to "assumed" makes sense. What we were returning were not freeable. Butthe caller does not free it; what you did is merely to shift the leakage from here to its caller. Is it worth the churn and one extra allocation to still leak slightly (namely, by sizeof(size_t)) smaller chunk of memory than what the code currently does? I doubt it.
When help.autocorrect is not set, we exit()'d without free'ing it like we do when the config is set; fix this.
I don't see any point in doing this, immediately in the same function on the previous line of exit(1).
quoted hunk
Signed-off-by: Tay Ray Chuan <redacted> --- Changed in v2: plug leak when help.autocorrect is not set. --- help.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)diff --git a/help.c b/help.c index dfb2e9d..ee261f4 100644 --- a/help.c +++ b/help.c@@ -362,8 +362,7 @@ const char *help_unknown_cmd(const char *cmd) ; /* still counting */ } if (autocorrect && n == 1 && SIMILAR_ENOUGH(best_similarity)) { - const char *assumed = main_cmds.names[0]->name; - main_cmds.names[0] = NULL; + const char *assumed = xstrdup(main_cmds.names[0]->name); clean_cmdnames(&main_cmds); fprintf_ln(stderr, _("WARNING: You called a Git command named '%s', "@@ -390,6 +389,7 @@ const char *help_unknown_cmd(const char *cmd) fprintf(stderr, "\t%s\n", main_cmds.names[i]->name); } + clean_cmdnames(&main_cmds); exit(1); }