remote: always allocate branch.push_tracking_ref

In branch_get_push(), we usually allocate a new string for the @{push}
ref, but will not do so in push.default=upstream mode, where we just
pass back the result of branch_get_upstream() directly.

This led to a hacky memory management scheme in e291c75a95 (remote.c:
add branch_get_push, 2015-05-21): we store the result in the
push_tracking_ref field of a "struct branch", under the assumption that
the branch struct will last until the end of the program. So even though
the struct doesn't know if it has an allocated string or not, it doesn't
matter because we hold on to it either way.

But that assumption was violated by f5ccb535cc (remote: fix leaking
config strings, 2024-08-22), which added a function to free branch
structs. Any struct which is fed to branch_release() is at risk of
leaking its push_tracking_ref member.

I don't think this can actually be triggered in practice. We rarely
actually free the branch structs, and we only fill in the
push_tracking_ref string lazily when it is needed. So triggering the
leak would require a code path that does both, and I couldn't find one.

Still, this is an ugly trap that may eventually spring on us. Since
there is only one code path in branch_get_push() that doesn't allocate,
let's just have it copy the string. And then we know that
push_tracking_ref is always allocated, and we can free it in
branch_release().

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 041f9ceb527b0e6a698bc87695e6e29a47e36612..c61bcc905f900d5deb50fdb39262f23a8c1e0f5b 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -272,6 +272,7 @@ static void branch_release(struct branch *branch)
        free((char *)branch->refname);
        free(branch->remote_name);
        free(branch->pushremote_name);
+       free(branch->push_tracking_ref);
        merge_clear(branch);
 }
 
@@ -1890,8 +1891,8 @@ static char *tracking_for_push_dest(struct remote *remote,
        return ret;
 }
 
-static const char *branch_get_push_1(struct repository *repo,
-                                    struct branch *branch, struct strbuf *err)
+static char *branch_get_push_1(struct repository *repo,
+                              struct branch *branch, struct strbuf *err)
 {
        struct remote_state *remote_state = repo->remote_state;
        struct remote *remote;
@@ -1931,7 +1932,7 @@ static const char *branch_get_push_1(struct repository *repo,
                return tracking_for_push_dest(remote, branch->refname, err);
 
        case PUSH_DEFAULT_UPSTREAM:
-               return branch_get_upstream(branch, err);
+               return xstrdup_or_null(branch_get_upstream(branch, err));
 
        case PUSH_DEFAULT_UNSPECIFIED:
        case PUSH_DEFAULT_SIMPLE:

diff --git a/remote.h b/remote.h
index 0ca399e1835bf1829054d8937d95e5625c38d881..fc052945ee451d35510fe90404c6a7dcd0ab5b0e 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -331,7 +331,7 @@ struct branch {
 
        int merge_alloc;
 
-       const char *push_tracking_ref;
+       char *push_tracking_ref;
 };
 
 struct branch *branch_get(const char *name);