On Sat, Apr 06, 2024 at 10:53:09AM -0700, Junio C Hamano wrote:
> Rubén Justo <rjusto@xxxxxxxxx> 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.
