Re: [PATCH 2/2] sequencer.c: plug mem leak in git_sequencer_config

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 21, 2018, 07:03 UTC
Message-ID: <nycvar.QRO.7.76.6.1806210857520.11870@tvgsbejvaqbjf.bet>
In-Reply-To: <20180604045122.GE14451@sigill.intra.peff.net>

Hi Peff,

On Mon, 4 Jun 2018, Jeff King wrote:

Show 15 quoted lines

> On Mon, Jun 04, 2018 at 01:26:57PM +0900, Junio C Hamano wrote:
> 
> > And at that point, maybe
> > 
> > 	char *some_var = xstrdup("default");
> > 	git_config_string(&some_var, ...);
> > 
> > that takes "char **" and frees the current storage before assigning to
> > it may be simpler than the two-variable approach.
> 
> That _is_ much nicer, but you cannot use xstrdup() as the initializer
> for a global "static char *some_var", which is what the majority of the
> config variables are. It's this "static initializer sometimes, run-time
> heap sometimes" duality to the variables that makes handling it such a
> pain.

This makes me think of Michael's proposal to teach strbuf some sort of STRBUF_INIT_CONST("default") which would set the appropriate len and set alloc to 0.

That way, we could turn those settings into strbufs that only allocate memory when/if needed.

We would need to be careful to revisit all strbuf_*() functions to make sure that they test not only for enough space when growing the string, but also when reducing it (unless reducing to len 0, in which case we can reassign strbuf_slopbuf to the buf field).

We also would need to revisit strbuf_*() to understand alloc < len to mean that we cannot realloc(), but have to malloc() && memcpy().

As a bonus, the pattern used for

[section]
	var = 1
	var = 2
	var = 3

would change from three times strdup() and two times free() to one time malloc().

Ciao, Dscho