On Thu, Feb 02 2023, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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.