Re: [PATCH v2 3/4] help.c: plug leaks with(out) help.autocorrect

[Junio C Hamano <gitster@pobox.com>]

Tay Ray Chuan <rctay89@gmail.com> 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).

> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
> ---
>
> 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);
>  }