Re: [PATCH v2 23/25] name-rev: avoid leaking memory in the `deref` case

From: Junio C Hamano <gitster@pobox.com>
Date: May 2, 2017, 03:26 UTC
Message-ID: <xmqqd1bsdjmw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <9e897d6f263bcf9a2b2bab613946fe1bf452eb91.1493387231.git.johannes.schindelin@gmx.de>

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> When the `name_rev()` function is asked to dereference the tip name, it
> allocates memory. But when it turns out that another tip already
> described the commit better than the current one, we forgot to release
> the memory.

Very well explained.

Show 41 quoted lines

>
> Pointed out by Coverity.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  builtin/name-rev.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index 92a5d8a5d26..a4ce73fb1e9 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -28,6 +28,7 @@ static void name_rev(struct commit *commit,
>  	struct rev_name *name = (struct rev_name *)commit->util;
>  	struct commit_list *parents;
>  	int parent_number = 1;
> +	char *p = NULL;
>  
>  	parse_commit(commit);
>  
> @@ -35,7 +36,7 @@ static void name_rev(struct commit *commit,
>  		return;
>  
>  	if (deref) {
> -		tip_name = xstrfmt("%s^0", tip_name);
> +		tip_name = p = xstrfmt("%s^0", tip_name);
>  
>  		if (generation)
>  			die("generation: %d, but deref?", generation);
> @@ -53,8 +54,10 @@ static void name_rev(struct commit *commit,
>  		name->taggerdate = taggerdate;
>  		name->generation = generation;
>  		name->distance = distance;
> -	} else
> +	} else {
> +		free(p);
>  		return;
> +	}
>  
>  	for (parents = commit->parents;
>  			parents;