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>

diff --git a/ref-filter.c b/ref-filter.c
index 370cc5b44a68357b0a4fb1acfd37c42f51889695..0f51095bbd4391d5a39be376acbcae44025c69b7 100644 (file)
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2237,7 +2237,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname,
                const char *merge;
 
                merge = remote_ref_for_branch(branch, atom->u.remote_ref.push);
-               *s = xstrdup(merge ? merge : "");
+               *s = merge ? merge : xstrdup("");
        } else
                BUG("unhandled RR_* enum");
 }

diff --git a/remote.c b/remote.c
index 8f3dee13186e7c539c077b81d123b7b50131300e..539e5ceae30ed7294f1733d4b9ac000bb671245b 100644 (file)
--- a/remote.c
+++ b/remote.c
@@ -632,7 +632,7 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit)
 static struct remote *remotes_remote_get(struct remote_state *remote_state,
                                         const char *name);
 
-const char *remote_ref_for_branch(struct branch *branch, int for_push)
+char *remote_ref_for_branch(struct branch *branch, int for_push)
 {
        read_config(the_repository, 0);
        die_on_missing_branch(the_repository, branch);
@@ -640,11 +640,11 @@ const char *remote_ref_for_branch(struct branch *branch, int for_push)
        if (branch) {
                if (!for_push) {
                        if (branch->merge_nr) {
-                               return branch->merge_name[0];
+                               return xstrdup(branch->merge_name[0]);
                        }
                } else {
-                       const char *dst,
-                               *remote_name = remotes_pushremote_for_branch(
+                       char *dst;
+                       const char *remote_name = remotes_pushremote_for_branch(
                                        the_repository->remote_state, branch,
                                        NULL);
                        struct remote *remote = remotes_remote_get(

diff --git a/remote.h b/remote.h
index b901b56746dfec31a54d1c7b0fca10958d860e4a..a58713f20ad654bc2ce441f67d39c315b12541bf 100644 (file)
--- a/remote.h
+++ b/remote.h
@@ -329,7 +329,7 @@ struct branch {
 struct branch *branch_get(const char *name);
 const char *remote_for_branch(struct branch *branch, int *explicit);
 const char *pushremote_for_branch(struct branch *branch, int *explicit);
-const char *remote_ref_for_branch(struct branch *branch, int for_push);
+char *remote_ref_for_branch(struct branch *branch, int for_push);
 
 /* returns true if the given branch has merge configuration given. */
 int branch_has_merge_config(struct branch *branch);