Subject: Re: [PATCH v6 19/19] push: free_refs() the "local_refs" in set_refspecs()

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 06 Feb 2023 18:19:14 +0100

On Thu, Feb 02 2023, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <[email-masked]> writes:
>
>> Fix a memory leak that's been with us since this code was added in
>> ca02465b413 (push: use remote.$name.push as a refmap, 2013-12-03).
>>
>> The "remote = remote_get(...)" added in the same commit would seem to
>> leak based only on the context here, but that function is a wrapper
>> for sticking the remotes we fetch into "the_repository->remote_state".
>>
>> See fd3cb0501e1 (remote: move static variables into per-repository
>> struct, 2021-11-17) for the addition of code in repository.c that
>> free's the "remote" allocated here.
>
> As I noted in my review of the previous step, the "if !local_heads"
> change we saw in the previous step goes better in this step, whose
> focus is on fixing the local_refs leak.

As I noted in the reply there it belongs in the preceding commit because
we currently *don't* allocate in a loop, it just looks at first glance
as though we might.

In practice we have earlier checks that would have died if the
remote_get() died, so the current "remote" sentinel value always
includes "local_heads".

I'll send a re-roll of this sooner than later with something to address
your 17/19 comment, but am keeping 18-19/19 the same, unless you reply
here/disagree/I've missed something.

Thanks.