@matheusbsbs / matheus-bagliano / commits / aa561208d9

push: refactor refspec_append_mapped() for subsequent leak-fix

The set_refspecs() caller of refspec_append_mapped() (added in [1]) left open the question[2] of whether the "remote" we lazily fetch might be NULL in the "[...]uniquely name our ref?" case, as remote_get() can return NULL. If we got past the "[...]uniquely name our ref?" case we'd have already segfaulted if we tried to dereference it as "remote->push.nr". In these cases the config mechanism & previous remote validation will have bailed out earlier. Let's refactor this code to clarify that, we'll now BUG() out if we can't get a "remote", and will no longer retrieve it for these common cases where we don't need it. 1. ca02465b413 (push: use remote.$name.push as a refmap, 2013-12-03) 2. https://lore.kernel.org/git/c0c07b89-7eaf-21cd-748e-e14ea57f09fd@web.de/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 7, 2023 at 00:07 UTC aa561208d9d41d757ede1fb40d3ae2c164a21f97
1 file changed +17 -12
builtin/push.c
+17 -12
@@ -63,16 +63,9 @@ static struct refspec rs = REFSPEC_INIT_PUSH;
63 static struct string_list push_options_config = STRING_LIST_INIT_DUP;
64
65 static void refspec_append_mapped(struct refspec *refspec, const char *ref,
66 - struct remote *remote, struct ref *local_refs)
66 + struct remote *remote, struct ref *matched)
67 {
68 const char *branch_name;
69 - struct ref *matched = NULL;
70 -
71 - /* Does "ref" uniquely name our ref? */
72 - if (count_refspec_match(ref, local_refs, &matched) != 1) {
73 - refspec_append(refspec, ref);
74 - return;
75 - }
69
70 if (remote->push.nr) {
71 struct refspec_item query;
@@ -120,12 +113,24 @@ static void set_refspecs(const char **refs, int nr, const char *repo)
113 die(_("--delete only accepts plain target ref names"));
114 refspec_appendf(&rs, ":%s", ref);
115 } else if (!strchr(ref, ':')) {
123 - if (!remote) {
124 - /* lazily grab remote and local_refs */
125 - remote = remote_get(repo);
116 + struct ref *matched = NULL;
117 +
118 + /* lazily grab local_refs */
119 + if (!local_refs)
120 local_refs = get_local_heads();
121 +
122 + /* Does "ref" uniquely name our ref? */
123 + if (count_refspec_match(ref, local_refs, &matched) != 1) {
124 + refspec_append(&rs, ref);
125 + } else {
126 + /* lazily grab remote */
127 + if (!remote)
128 + remote = remote_get(repo);
129 + if (!remote)
130 + BUG("must get a remote for repo '%s'", repo);
131 +
132 + refspec_append_mapped(&rs, ref, remote, matched);
133 }
128 - refspec_append_mapped(&rs, ref, remote, local_refs);
134 } else
135 refspec_append(&rs, ref);
136 }