Re: [PATCH 0/2] dir & unpak-trees: memory-leak fixes

From: Elijah Newren <newren@gmail.com>
Date: Oct 6, 2021, 16:33 UTC
Message-ID: <CABPp-BGsJOtffyLqk85Z65chJkPr+PsZieK=eX57ccKawdVAVQ@mail.gmail.com>
In-Reply-To: <cover-0.2-00000000000-20211006T093405Z-avarab@gmail.com>

On Wed, Oct 6, 2021 at 2:40 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

>
> These couple of patches fix some remaining memory leaks in
> unpack-trees.c, which mostly happen via a lack of calls to dir_clear()
> fixed in 2/2.

There is no added dir_clear() in 2/2 (the equivalent patch in the previous series you took that patch from did have one, so I think this was just an incomplete edit-or-commenting-after-rebasing case). More comments on that patch. The idea for 2/2 is probably good, but should probably be accompanied with another tweak, and the commit message and cover letter about it need some updating.

Patch 1/2 is a clear memory leak fix.

Show 32 quoted lines

> This goes on top of ab/sanitize-leak-ci (but not
> en/removing-untracked-fixes). In 1/2 I mark a test as passing under
> SANITIZE=leak, which assumes that an environment variable added by
> ab/sanitize-leak-ci is understood. Without ab/sanitize-leak-ci it'll
> be silently ignored.
>
> Elijah has parallel work in fixing leaks in dir.c, but without his
> ab/sanitize-leak-ci that test will also pass, it's not one of the ones
> that needed something like his leak fixes to {dir,unpack-trees}.c to
> pass.
>
> Ævar Arnfjörð Bjarmason (2):
>   unpack-trees: don't leak memory in verify_clean_subdirectory()
>   built-ins & lib: plug memory leaks with unpack_trees_options_release()
>
>  archive.c                   | 11 ++++++++---
>  builtin/am.c                | 17 ++++++++++++-----
>  builtin/checkout.c          |  9 +++++++--
>  builtin/clone.c             |  1 +
>  builtin/commit.c            |  6 +++++-
>  builtin/merge.c             |  6 ++++--
>  builtin/read-tree.c         | 14 ++++++++++----
>  builtin/reset.c             | 13 +++++++++----
>  builtin/stash.c             | 14 ++++++++++----
>  diff-lib.c                  |  5 ++++-
>  sequencer.c                 |  2 ++
>  t/t1001-read-tree-m-2way.sh |  2 ++
>  unpack-trees.c              |  3 ++-
>  13 files changed, 76 insertions(+), 27 deletions(-)
>
> --
> 2.33.0.1441.gbbcdb4c3c66