Re: [PATCH 09/20] name-rev: don't xstrdup() an already dup'd string

From: René Scharfe <l.s.r@web.de>
Date: Dec 28, 2022, 20:31 UTC
Message-ID: <dbc7c55d-d546-d004-ef44-62bd7349d5c9@web.de>
In-Reply-To: <patch-09.20-77fcdeb9284-20221228T175512Z-avarab@gmail.com>

Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:

Show 26 quoted lines

> When "add_to_tip_table()" is called with a non-zero
> "shorten_unambiguous" we always return an xstrdup()'d string, which
> we'd then xstrdup() again, leaking memory. See [1] and [2] for how
> this leak came about.
>
> 1. 98c5c4ad015 (name-rev: allow to specify a subpath for --refs
>    option, 2013-06-18)
> 2. b23e0b9353e (name-rev: allow converting the exact object name at
>    the tip of a ref, 2013-07-07)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  builtin/name-rev.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 15535e914a6..24f4438eb01 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -313,7 +313,8 @@ static void add_to_tip_table(const struct object_id *oid, const char *refname,
>
>  	ALLOC_GROW(tip_table.table, tip_table.nr + 1, tip_table.alloc);
>  	oidcpy(&tip_table.table[tip_table.nr].oid, oid);
> -	tip_table.table[tip_table.nr].refname = xstrdup(refname);
> +	tip_table.table[tip_table.nr].refname = shorten_unambiguous ? refname :
> +		xstrdup(refname);

Hmm, this works based on knowledge about the inner workings of name_ref_abbrev(), which provides the refname. Could be cleaned up by inlining that short function, or by moving the xstrdup() call there.

>  	tip_table.table[tip_table.nr].commit = commit;
>  	tip_table.table[tip_table.nr].taggerdate = taggerdate;
>  	tip_table.table[tip_table.nr].from_tag = from_tag;