On 21/06/2021 23:54, Elijah Newren wrote:
On Sun, Jun 20, 2021 at 8:14 AM <andrzej@xxxxxxxxx> wrote:
From: Andrzej Hunt <andrzej@xxxxxxxxx>
This series plugs more of the leaks that were found while running
t0002-t0099 with LSAN.
See also the first series (already merged) at [1]. I'm currently
expecting at least another 2 series before t0002-t0099 run leak free.
I'm not being particularly systematic about the order of patches -
although I am trying to send out "real" (if mostly small) leaks first,
before sending out the more boring patches that add free()/UNLEAK() to
cmd_* and direct helpers thereof.
I've read over the series. It provides some good clear fixes. I
noted on patches 2, 6, and 12 that a some greps suggested that leaks
similar to the ones being fixed likely also affect other places of the
codebase. Those other places don't need to be fixed as part of this
series, but they might be good items for #leftoverbits or GSoC early
tasks (cc: Christian in case he wants to record those somewhere).
I cc'ed Stolee on patch 4 because he suggested he wanted to read it in
an earlier discussion.
Phillip noted some issues with patch 11, and I added a couple more.
The ownership of opts->strategy appears to be pretty messy and in need
of cleanup.
All the patches other than 11 look good to me.
Thank you for the careful reviews - and especially for pointing out when
a given pattern does occur elsewhere in the codebase!
As suggested I will skip the additional locations that you've found
while reviewing this series - but I'm starting a separate series where I
can address those. I have been focused on getting tests to pass
leak-free one-by-one, but spotting and fixing patterns is probably more
efficient (since author and reviewer already have the right context) and
in some cases might fix leaks that aren't occurring during the tests.
ATB,
Andrzej
