Re: [PATCH v2 0/3] unpack-trees: memory-leak fixes

[Elijah Newren <newren@gmail.com>]

On Thu, Oct 7, 2021 at 2:46 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
> These patches fix a couple of stray memory leaks in unpack-trees.c.
>
> This goes on top of ab/sanitize-leak-ci (but not
> en/removing-untracked-fixes, although they combine to fix more leaks
> in the area).
>
> Changes since v1[1]:
>
> In rebasing v1 from some earlier patches I came up with something that
> didn't make sense with related fixes in Elijah's
> en/removing-untracked-fixes. This hopefully makes sense:
>
>  * The old 3/3 is gone, but there's a new 2-3/3 which fix the only
>    actual leak that was left, i.e. the one in sequencer.c.
>
>  * We might want something like the 3/3 from v1 of this series where
>    we call clear_unpack_trees_porcelain() everywhere (and rename it to
>    unpack_trees_options_release()) just for good measure and in case
>    we'd ever add something to the struct that needs unconditional
>    freeing.
>
>    But let's punt on that here and just keep the current
>    setup_unpack_trees_porcelain()/clear_unpack_trees_porcelain()
>    behavior, callers who don't use setup_unpack_trees_porcelain() but
>    use "struct unpack_trees_options" don't need to call any free-like
>    function at the end.
>
> 1. https://lore.kernel.org/git/cover-0.2-00000000000-20211006T093405Z-avarab@gmail.com/
>
> Ævar Arnfjörð Bjarmason (3):
>   unpack-trees: don't leak memory in verify_clean_subdirectory()
>   sequencer: add a "goto cleanup" to do_reset()
>   sequencer: fix a memory leak in do_reset()

Left a bunch of comments on patch 2.  The upshot is anywhere from "the
commit message just needs a few tweaks" to "well, maybe we should
clean up these other things too, and we could restructure some related
code...".  I'll let you pick where in that spectrum you want to take
things, because the other possible changes I mention beyond fixing the
commit message don't need to hold up this series.