On Thu, Mar 24 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> This is a re-roll of the v1 of [1] now that 7391ecd338e (Merge branch >> 'ds/partial-bundles', 2022-03-21) has landed, which it had a conflict >> with. I believe this v2 addresses all the feedback brought up on v1. > > Thanks. It was mostly a pleasant read but with a huge caveat---I > have no confidence in the code that would not try to release what it > did not allocate (simply because I do not have time to audit all > allocations to various members of rev-info structure). But at least > if we try to free something we borrowed from say command line, we'd > immediately get a crash so with enough cooking and guinea-pig testing, > such a bug would be easy to catch. Yes, especially with SANITIZE=address and friends. > I suspect cmd_show() still is leaky when fed a few commits. > [...] > There are ways to show different types of objects, but what we are > interested in is what happens to commits. > > case OBJ_COMMIT: > rev.pending.nr = rev.pending.alloc = 0; > rev.pending.objects = NULL; Yes, FWIW I know about that one, and will add some passing mention of it to a commit message. There's still a bunch of leaks related to the revisions.c code after this series, which doesn't address all of them. But since it's already getting up to ~30 patches I wanted to address the most common ones, this and various other tricky edge cases have been left for a follow-up.