From: René Scharfe <l.s.r@web.de>
Date: Sat, 08 Oct 2016 13:38:24 GMT
Subject: Re: [PATCH] remote.c: free previous results when looking for a ref match
Message-ID: <e9f58e04-a534-6794-feac-f6a47ca8bb65@web.de>
In-Reply-To: <20161007235857.20382-1-sbeller@google.com>

Am 08.10.2016 um 01:58 schrieb Stefan Beller:
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  remote.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/remote.c b/remote.c
> index ad6c542..5f9afb4 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -833,6 +833,8 @@ static int match_name_with_pattern(const char *key, const char *name,
>  		strbuf_add(&sb, value, vstar - value);
>  		strbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);
>  		strbuf_addstr(&sb, vstar + 1);
> +		if (*result)
> +			free(*result);

free(3) can handle NULL pointers; this check is not necessary.

Is it wise to release memory for callers?  I'd expect them to be 
responsible for that.  Some of them can pass uninitialized pointers; 
this is not allowed anymore after the change.

>  		*result = strbuf_detach(&sb, NULL);
>  	}
>  	return ret;
> @@ -1262,6 +1264,8 @@ static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref
>  		 */
>  		if (!send_mirror && !starts_with(ref->name, "refs/heads/"))
>  			return NULL;
> +		if (name)
> +			free(name);

Again, this check is not necessary.  If I read the code correctly the 
pointer could be uninitialized at that point, though, causing free(3) to 
crash.

>  		name = xstrdup(ref->name);
>  	}
>  	if (ret_pat)
>