[PATCH 1/2] ui-stats: free without condition

John Keeping john at keeping.me.uk
Mon Oct 12 11:10:12 CEST 2015


On Mon, Oct 12, 2015 at 10:59:34AM +0200, Christian Hesse wrote:
> From: Christian Hesse <mail at eworm.de>
> 
> xstrdup() returns allocated memory or NULL. It's safe to call free()
> without condition.
> 
> Coverity-Id 13839 is kind of false posivtive, but this should fix it
> nevertheless.
> 
> Coverity-Id: 13839
> Signed-off-by: Christian Hesse <mail at eworm.de>
> ---
>  ui-stats.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

This is wrong - we don't have strdup_strings set in the string_list so
it takes ownership of the pointer.  The test on item->util is used as a
proxy testing if the entry is newly added to the list (in which case it
has taken ownership of the string) or not (in which case we must free
the string).

> diff --git a/ui-stats.c b/ui-stats.c
> index 74ce0f7..02335e7 100644
> --- a/ui-stats.c
> +++ b/ui-stats.c
> @@ -180,8 +180,7 @@ static void add_commit(struct string_list *authors, struct commit *commit,
>  	author = string_list_insert(authors, tmp);
>  	if (!author->util)
>  		author->util = xcalloc(1, sizeof(struct authorstat));
> -	else
> -		free(tmp);
> +	free(tmp);
>  	authorstat = author->util;
>  	items = &authorstat->list;
>  	t = info->committer_date;
> @@ -189,8 +188,7 @@ static void add_commit(struct string_list *authors, struct commit *commit,
>  	period->trunc(date);
>  	tmp = xstrdup(period->pretty(date));
>  	item = string_list_insert(items, tmp);
> -	if (item->util)
> -		free(tmp);
> +	free(tmp);
>  	item->util++;
>  	authorstat->total++;
>  	cgit_free_commitinfo(info);
> -- 
> 2.6.1


More information about the CGit mailing list