{"thread":{"id":"44250","subject":"[PATCH] remote.c: free previous results when looking for a ref match","startedAt":"2016-10-08T00:04:13Z","lastAt":"2016-10-10T17:03:28Z","messageCount":3,"participants":["Stefan Beller","René Scharfe"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"303624","messageId":"20161007235857.20382-1-sbeller@google.com","threadId":"44250","inReplyTo":null,"subject":"[PATCH] remote.c: free previous results when looking for a ref match","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-07T23:58:57Z","receivedAt":"2016-10-08T00:04:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n remote.c | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/remote.c b/remote.c\nindex ad6c542..5f9afb4 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -833,6 +833,8 @@ static int match_name_with_pattern(const char *key, const char *name,\n \t\tstrbuf_add(&sb, value, vstar - value);\n \t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n \t\tstrbuf_addstr(&sb, vstar + 1);\n+\t\tif (*result)\n+\t\t\tfree(*result);\n \t\t*result = strbuf_detach(&sb, NULL);\n \t}\n \treturn ret;\n@@ -1262,6 +1264,8 @@ static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref\n \t\t */\n \t\tif (!send_mirror && !starts_with(ref->name, \"refs/heads/\"))\n \t\t\treturn NULL;\n+\t\tif (name)\n+\t\t\tfree(name);\n \t\tname = xstrdup(ref->name);\n \t}\n \tif (ret_pat)\n-- \n2.10.1.353.g1629400\n\n"},{"id":"303645","messageId":"e9f58e04-a534-6794-feac-f6a47ca8bb65@web.de","threadId":"44250","inReplyTo":"20161007235857.20382-1-sbeller@google.com","subject":"Re: [PATCH] remote.c: free previous results when looking for a ref match","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-10-08T13:38:24Z","receivedAt":"2016-10-08T13:38:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 08.10.2016 um 01:58 schrieb Stefan Beller:\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  remote.c | 4 ++++\n>  1 file changed, 4 insertions(+)\n>\n> diff --git a/remote.c b/remote.c\n> index ad6c542..5f9afb4 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -833,6 +833,8 @@ static int match_name_with_pattern(const char *key, const char *name,\n>  \t\tstrbuf_add(&sb, value, vstar - value);\n>  \t\tstrbuf_add(&sb, name + klen, namelen - klen - ksuffixlen);\n>  \t\tstrbuf_addstr(&sb, vstar + 1);\n> +\t\tif (*result)\n> +\t\t\tfree(*result);\n\nfree(3) can handle NULL pointers; this check is not necessary.\n\nIs it wise to release memory for callers?  I'd expect them to be \nresponsible for that.  Some of them can pass uninitialized pointers; \nthis is not allowed anymore after the change.\n\n>  \t\t*result = strbuf_detach(&sb, NULL);\n>  \t}\n>  \treturn ret;\n> @@ -1262,6 +1264,8 @@ static char *get_ref_match(const struct refspec *rs, int rs_nr, const struct ref\n>  \t\t */\n>  \t\tif (!send_mirror && !starts_with(ref->name, \"refs/heads/\"))\n>  \t\t\treturn NULL;\n> +\t\tif (name)\n> +\t\t\tfree(name);\n\nAgain, this check is not necessary.  If I read the code correctly the \npointer could be uninitialized at that point, though, causing free(3) to \ncrash.\n\n>  \t\tname = xstrdup(ref->name);\n>  \t}\n>  \tif (ret_pat)\n>\n\n"},{"id":"303735","messageId":"CAGZ79kZ3n8iNqLrNA0WOMEs7zLTMHmLxc-WAo_fgj1uLQtwv5g@mail.gmail.com","threadId":"44250","inReplyTo":"e9f58e04-a534-6794-feac-f6a47ca8bb65@web.de","subject":"Re: [PATCH] remote.c: free previous results when looking for a ref match","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-10-10T17:03:22Z","receivedAt":"2016-10-10T17:03:28Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Oct 8, 2016 at 6:38 AM, René Scharfe <l.s.r@web.de> wrote:\n\n>\n> Again, this check is not necessary.  If I read the code correctly the\n> pointer could be uninitialized at that point, though, causing free(3) to\n> crash.\n\nyep, this patch is bogus.\n"}]}