Subject: Re: [PATCH v6 18/19] push: refactor refspec_append_mapped() for subsequent leak-fix

From: Junio C Hamano <[email-masked]>
Date: Thu, 02 Feb 2023 14:10:14 -0800

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

> 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.

Another thing this does, if I am not mistaken, and the above does
not mention, is, that the old code called get_local_heads() for each
and every ref from the command line, and the second and subsequent
calls to the function and assignment to local_refs would leak the
entire local_refs linked list.  This step does not release the
linked list at the end, but at least it stops leaking it in each
iteration of the loop.