ref-filter: fix leak when formatting %(push:remoteref)

When we expand the %(upstream) or %(push) placeholders, we rely on remote.c's remote_ref_for_branch() to fill in the ":refname" argument. But that function has confusing memory ownership semantics: it may or may not return an allocated string, depending on whether we are in "upstream" mode or "push" mode. The caller in ref-filter.c always duplicates the result, meaning that we leak the original in the case of %(push:refname). To solve this, let's make the return value from remote_ref_for_branch() consistent, by always returning an allocated pointer. Note that the switch to returning a non-const pointer has a ripple effect inside the function, too. We were storing the "dst" result as a const pointer, too, even though it is always allocated! It is the return value from apply_refspecs(), which is always a non-const allocated string. And then on the caller side in ref-filter.c (and this is the only caller at all), we just need to avoid the extra duplication when the return value is non-NULL. This clears up one case that LSan finds in t6300, but there are more. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 9, 2024 at 19:19 UTC f046127b6682f98d41bb4d26164da7f1a4a8e8d0
3 files changed +6 -6
ref-filter.c
+1 -1
@@ -2237,7 +2237,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
2237 const char *merge;
2238
2239 merge = remote_ref_for_branch(branch, atom->u.remote_ref.push);
2240 - *s = xstrdup(merge ? merge : "");
2240 + *s = merge ? merge : xstrdup("");
2241 } else
2242 BUG("unhandled RR_* enum");
2243 }
remote.c
+4 -4
@@ -632,7 +632,7 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
632 static struct remote *remotes_remote_get(struct remote_state *remote_state,
633 const char *name);
634
635 -const char *remote_ref_for_branch(struct branch *branch, int for_push)
635 +char *remote_ref_for_branch(struct branch *branch, int for_push)
636 {
637 read_config(the_repository, 0);
638 die_on_missing_branch(the_repository, branch);
@@ -640,11 +640,11 @@ const char *remote_ref_for_branch(struct branch *branch, int for_push)
640 if (branch) {
641 if (!for_push) {
642 if (branch->merge_nr) {
643 - return branch->merge_name[0];
643 + return xstrdup(branch->merge_name[0]);
644 }
645 } else {
646 - const char *dst,
647 - *remote_name = remotes_pushremote_for_branch(
646 + char *dst;
647 + const char *remote_name = remotes_pushremote_for_branch(
648 the_repository->remote_state, branch,
649 NULL);
650 struct remote *remote = remotes_remote_get(
remote.h
+1 -1
@@ -329,7 +329,7 @@ struct branch {
329 struct branch *branch_get(const char *name);
330 const char *remote_for_branch(struct branch *branch, int *explicit);
331 const char *pushremote_for_branch(struct branch *branch, int *explicit);
332 -const char *remote_ref_for_branch(struct branch *branch, int for_push);
332 +char *remote_ref_for_branch(struct branch *branch, int for_push);
333
334 /* returns true if the given branch has merge configuration given. */
335 int branch_has_merge_config(struct branch *branch);