From d79fff4a11a527f57516c62fe00777852bab719a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 19 Jan 2026 00:23:20 -0500 Subject: [PATCH] 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 Signed-off-by: Junio C Hamano --- remote.c | 7 ++++--- remote.h | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/remote.c b/remote.c index 041f9ceb52..c61bcc905f 100644 --- 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 0ca399e183..fc052945ee 100644 --- 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);