From: Junio C Hamano Date: Thu, 02 Feb 2023 22:10:14 GMT Subject: Re: [PATCH v6 18/19] push: refactor refspec_append_mapped() for subsequent leak-fix Message-ID: In-Reply-To: Ævar Arnfjörð Bjarmason 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.