Skip to content

Commit d79fff4

Browse files
peffgitster
authored andcommitted
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 e291c75 (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 f5ccb53 (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>
1 parent 9bf9eed commit d79fff4

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

remote.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@ static void branch_release(struct branch *branch)
272272
free((char *)branch->refname);
273273
free(branch->remote_name);
274274
free(branch->pushremote_name);
275+
free(branch->push_tracking_ref);
275276
merge_clear(branch);
276277
}
277278

@@ -1890,8 +1891,8 @@ static char *tracking_for_push_dest(struct remote *remote,
18901891
return ret;
18911892
}
18921893

1893-
static const char *branch_get_push_1(struct repository *repo,
1894-
struct branch *branch, struct strbuf *err)
1894+
static char *branch_get_push_1(struct repository *repo,
1895+
struct branch *branch, struct strbuf *err)
18951896
{
18961897
struct remote_state *remote_state = repo->remote_state;
18971898
struct remote *remote;
@@ -1931,7 +1932,7 @@ static const char *branch_get_push_1(struct repository *repo,
19311932
return tracking_for_push_dest(remote, branch->refname, err);
19321933

19331934
case PUSH_DEFAULT_UPSTREAM:
1934-
return branch_get_upstream(branch, err);
1935+
return xstrdup_or_null(branch_get_upstream(branch, err));
19351936

19361937
case PUSH_DEFAULT_UNSPECIFIED:
19371938
case PUSH_DEFAULT_SIMPLE:

remote.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ struct branch {
331331

332332
int merge_alloc;
333333

334-
const char *push_tracking_ref;
334+
char *push_tracking_ref;
335335
};
336336

337337
struct branch *branch_get(const char *name);

0 commit comments

Comments
 (0)