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>

2 files changed, 12 insertions, 1 deletions

diff --git a/remote.c b/remote.c
index fc5894e949..041f9ceb52 100644
--- 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 e3ad19298a..02fb92e99e 100644
--- 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 &&