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

2016-10-10 Thread Stefan Beller
On Sat, Oct 8, 2016 at 6:38 AM, René Scharfe  wrote:

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

yep, this patch is bogus.


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

2016-10-08 Thread René Scharfe

Am 08.10.2016 um 01:58 schrieb Stefan Beller:

Signed-off-by: Stefan Beller 
---
 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(, value, vstar - value);
strbuf_add(, name + klen, namelen - klen - ksuffixlen);
strbuf_addstr(, 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(, 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)