Re: [PATCH] remote.c: free previous results when looking for a ref match

From: René Scharfe <l.s.r@web.de>
Date: Oct 8, 2016, 13:38 UTC
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:

Show 15 quoted lines

> 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.

Show 9 quoted lines

>  		*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)
>