Re: [PATCH 20/23] builtin/maintenance: fix leaking config string

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

Patrick Steinhardt <ps@pks.im> writes:

> When parsing the maintenance strategy from config we allocate a config
> string, but do not free it after parsing it. Plug this leak by instead
> using `git_config_get_string_tmp()`, which does not allocate any memory.
>
> This leak is exposed by t7900, but plugging it alone does not make the
> test suite pass.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/gc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/gc.c b/builtin/gc.c
> index 7dac9714054..3acfa367ad1 100644
> --- a/builtin/gc.c
> +++ b/builtin/gc.c
> @@ -1476,9 +1476,9 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
>  
>  static void initialize_maintenance_strategy(void)
>  {
> -	char *config_str;
> +	const char *config_str;
>  
> -	if (git_config_get_string("maintenance.strategy", &config_str))
> +	if (git_config_get_string_tmp("maintenance.strategy", &config_str))
>  		return;

I am not a huge fan of the name of this function, because many
callers use the result as a fairly long-lived string (the primary
reason why they call them is to avoid having to free() the result at
the end), but this new caller's use is very much in line with the
"tmp" in its name.  We peek into the configset to learn the value of
this string, and check its value against "incremental".

Looking good.

Thanks.