Re: [PATCH 0/4] fix a leak with excludes_file

["Rubén Justo" <rjusto@gmail.com>]

On Sat, Apr 06, 2024 at 10:53:09AM -0700, Junio C Hamano wrote:
> Rubén Justo <rjusto@gmail.com> writes:
> 
> > We call twice to git_config(): first to get the main git-branch(1) options,
> > and second to get the ones related to the sequencer.

Obviously I meant git-rebase(1) -- or any other command that uses the
sequencer under the hood.

> Adding a parallel API
> next to the existing git_config_pathname() and interpolate_path()
> and convert only these callers that touch excludes_file would not
> help other callers that hold the pointer git_config_pathname()
> returned.

It does not have to be like that.  We may no longer need the current
and problematic git_config_pathname().  However I did not want to go
that far in this series.

>  config.c         | 4 +++-
>  t/t7300-clean.sh | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git c/config.c w/config.c
> index eebce8c7e0..ae3652b08f 100644
> --- c/config.c
> +++ w/config.c
> @@ -1584,8 +1584,10 @@ static int git_default_core_config(const char *var, const char *value,
>  	if (!strcmp(var, "core.askpass"))
>  		return git_config_string(&askpass_program, var, value);
>  
> -	if (!strcmp(var, "core.excludesfile"))
> +	if (!strcmp(var, "core.excludesfile")) {
> +		free((char *)excludes_file);

Aaah, you prefer this :-( ...  this free is what I was referring to, in
the message you are replying to, as the simpler fix ...

It obviously plugs the leak and so my itch is gone;  therefore OK by me
to the series I have seen that you have already sent with this.

Still, I find the approach in this series more interesting than the
simple free.  The strbuf saves us from subsequent free+alloc's, for
exactly the same "core.excludesFile".  The strbuf states clearly, IMHO,
who owns the pointer.  And other niceties.