remote: fix leak in branch_get_push_1() with invalid "simple" config

Most of the code paths in branch_get_push_1() allocate a string for the
@{push} value. We then return the result, which is stored in a "struct
branch", so the value is not leaked.

But there's one path that does leak: when we are in the "simple" push
mode, we have to check that the @{push} value matches what we'd get for
@{upstream}. If it doesn't, we return an error, but forget to free the
@{push} value we computed.

Curiously, the existing tests don't trigger this with LSan, even though
they do exercise the code path. As far as I can tell, it should be
triggered via:

  git -c push.default=simple \
      -c branch.foo.remote=origin \
      -c branch.foo.merge=refs/heads/not-foo \
      rev-parse foo@{push}

which will complain that the upstream ("not-foo") does not match the
push destination ("foo"). We do die() shortly after this, but not until
after returning from branch_get_push_1(), which is where the leak
happens.

So it seems like a false negative in LSan. However, I can trigger it
reliably by printing the @{push} value using for-each-ref. This takes a
little more setup (because we need "foo" to actually exist to iterate
over it with for-each-ref), but we can piggy-back on the existing repo
config in t6300.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/remote.c b/remote.c
index fc5894e9496c9debecf41eebd66bb49e67ba69ee..041f9ceb527b0e6a698bc87695e6e29a47e36612 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -1945,9 +1945,11 @@ static const char *branch_get_push_1(struct repository *repo,
                        cur = tracking_for_push_dest(remote, branch->refname, err);
                        if (!cur)
                                return NULL;
-                       if (strcmp(cur, up))
+                       if (strcmp(cur, up)) {
+                               free(cur);
                                return error_buf(err,
                                                 _("cannot resolve 'simple' push to a single destination"));
+                       }
                        return cur;
                }
        }

diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh
index e3ad19298accdeeb393b65fceb9e6bf97541cc8b..02fb92e99e5475a3179475dcba732d09839c1a6e 100644 (file)
--- a/t/for-each-ref-tests.sh
+++ b/t/for-each-ref-tests.sh
@@ -1744,6 +1744,15 @@ test_expect_success ':remotename and :remoteref' '
        )
 '
 
+test_expect_success '%(push) with an invalid push-simple config' '
+       echo "refs/heads/main " >expect &&
+       git -c push.default=simple \
+           -c remote.pushdefault=myfork \
+           for-each-ref \
+           --format="%(refname) %(push)" refs/heads/main >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success "${git_for_each_ref} --ignore-case ignores case" '
        ${git_for_each_ref} --format="%(refname)" refs/heads/MAIN >actual &&
        test_must_be_empty actual &&