Subject: Re: [PATCH 13/20] sideband: fix leaks when configuring sideband colors

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 22 Aug 2024 10:19:58 +0200

On Tue, Aug 20, 2024 at 04:52:59PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <[email-masked]> writes:
> 
> > We read a bunch of configs in `use_sideband_colors()` to configure the
> > colors that Git should use. We never free the strings read from the
> > config though, causing memory leaks. Fix those.
> >
> > Signed-off-by: Patrick Steinhardt <[email-masked]>
> > ---
> >  sideband.c                          | 8 +++++---
> >  t/t5409-colorize-remote-messages.sh | 1 +
> >  2 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/sideband.c b/sideband.c
> > index 5d8907151fe..deb6ec0a8b7 100644
> > --- a/sideband.c
> > +++ b/sideband.c
> > @@ -30,7 +30,7 @@ static int use_sideband_colors(void)
> >  
> >  	const char *key = "color.remote";
> >  	struct strbuf sb = STRBUF_INIT;
> > -	char *value;
> > +	char *value = NULL;
> 
> Hmph, this is a bit unfortunate.  If there is no configuration
> variable, on the first call to this function, we come to the end of
> if/else cascade and we need to free.
> 
> Another possibility to convey the intention better may be to assign
> NULL to value after the "we already know the value, so return early"
> took place.

There is an even better option: just use `git_config_get_string_tmp()`.
Then we don't have to worry about freeing the strings at all.

Patrick