remote: fix leaks when matching refspecs

In `match_explicit()`, we try to match a source ref with a destination
ref according to a refspec item. This matching sometimes requires us to
allocate a new source spec so that it looks like we expect. And while we
in some end up assigning this allocated ref as `peer_ref`, which hands
over ownership of it to the caller, in other cases we don't. We neither
free it though, causing a memory leak.

Fix the leak by creating a common exit path where we can easily free the
source ref in case it is allocated and hasn't been handed over to the
caller.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/remote.c b/remote.c
index 2c52119bbb2c60d4c4961cdb750e99f90709df91..6ea81f9665b5508805d039485f53113181a5b854 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1344,18 +1344,21 @@ static int match_explicit(struct ref *src, struct ref *dst,
                          struct ref ***dst_tail,
                          struct refspec_item *rs)
 {
-       struct ref *matched_src, *matched_dst;
-       int allocated_src;
+       struct ref *matched_src = NULL, *matched_dst = NULL;
+       int allocated_src = 0, ret;
 
        const char *dst_value = rs->dst;
        char *dst_guess;
 
-       if (rs->pattern || rs->matching || rs->negative)
-               return 0;
+       if (rs->pattern || rs->matching || rs->negative) {
+               ret = 0;
+               goto out;
+       }
 
-       matched_src = matched_dst = NULL;
-       if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)
-               return -1;
+       if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0) {
+               ret = -1;
+               goto out;
+       }
 
        if (!dst_value) {
                int flag;
@@ -1394,18 +1397,30 @@ static int match_explicit(struct ref *src, struct ref *dst,
                      dst_value);
                break;
        }
-       if (!matched_dst)
-               return -1;
-       if (matched_dst->peer_ref)
-               return error(_("dst ref %s receives from more than one src"),
-                            matched_dst->name);
-       else {
+
+       if (!matched_dst) {
+               ret = -1;
+               goto out;
+       }
+
+       if (matched_dst->peer_ref) {
+               ret = error(_("dst ref %s receives from more than one src"),
+                           matched_dst->name);
+               goto out;
+       } else {
                matched_dst->peer_ref = allocated_src ?
                                        matched_src :
                                        copy_ref(matched_src);
                matched_dst->force = rs->force;
+               matched_src = NULL;
        }
-       return 0;
+
+       ret = 0;
+
+out:
+       if (allocated_src)
+               free_one_ref(matched_src);
+       return ret;
 }
 
 static int match_explicit_refs(struct ref *src, struct ref *dst,

diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 08424e878e104cc19a43960b987cf868f542cad2..532035933f3326798741189300a1a6ef3b481c3f 100755 (executable)
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -2,6 +2,7 @@
 
 test_description='git remote porcelain-ish'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 setup_repository () {