[0/6] revisions API: fix more memory leaks

Message

Ævar Arnfjörð Bjarmason July 13, 2022, 1:10 p.m. UTC

This short series fixes a couple of mistakes in 1-2/6 that snuck in in
the revisions_release() series, those compliment the already queued
jk/diff-files-cleanup-fix.

The rest of this fixes a few tricky remaining memory leaks, allowing
us to mark the tests seen in the diffstat as passing with
SANITIZE=leak.

The "git show" leak in 4/6 in particular showed up in a lot of places
in our test suite, so fixing it really helps us to accelerate towards
marking more entire tests as leak-free.

Passing CI for this series can be found at:
https://github.com/avar/git/tree/avar/follow-up-release-revisions-fixes

Ævar Arnfjörð Bjarmason (6):
  bisect.c: add missing "goto" for release_revisions()
  test-fast-rebase helper: use release_revisions() (again)
  log: make the intent of cmd_show()'s "rev.pending" juggling clearer
  log: fix common "rev.pending" memory leak in "git show"
  bisect.c: partially fix bisect_rev_setup() memory leak
  revisions API: don't leak memory on argv elements that need free()-ing

 bisect.c                                      | 28 +++++++++++--------
 builtin/log.c                                 | 22 ++++++++-------
 builtin/submodule--helper.c                   |  5 +++-
 remote.c                                      |  5 +++-
 revision.c                                    |  2 ++
 revision.h                                    |  3 +-
 t/helper/test-fast-rebase.c                   |  2 --
 t/t0203-gettext-setlocale-sanity.sh           |  1 +
 t/t1020-subdirectory.sh                       |  1 +
 t/t2020-checkout-detach.sh                    |  1 +
 t/t3307-notes-man.sh                          |  1 +
 t/t3920-crlf-messages.sh                      |  2 ++
 t/t4069-remerge-diff.sh                       |  1 +
 t/t7007-show.sh                               |  1 +
 ...3-pre-commit-and-pre-merge-commit-hooks.sh |  1 +
 t/t9122-git-svn-author.sh                     |  1 -
 t/t9162-git-svn-dcommit-interactive.sh        |  1 -
 17 files changed, 50 insertions(+), 28 deletions(-)

Comments

Jeff King July 18, 2022, 3:14 p.m. UTC | #1

On Wed, Jul 13, 2022 at 03:10:29PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Ævar Arnfjörð Bjarmason (6):
>   bisect.c: add missing "goto" for release_revisions()
>   test-fast-rebase helper: use release_revisions() (again)
>   log: make the intent of cmd_show()'s "rev.pending" juggling clearer
>   log: fix common "rev.pending" memory leak in "git show"
>   bisect.c: partially fix bisect_rev_setup() memory leak
>   revisions API: don't leak memory on argv elements that need free()-ing

This looks correct overall to me. I left a few comments:

  - the readability tweaks in 3 and 4 I think are worth doing. If you
    agree, I'd like to see that in a re-roll.

  - the bigger "rev_info owns its argv" option for patches 5 and 6 is a
    good suggestion, I think, but maybe is a bigger change (though if
    it's optional, it shouldn't need to affect any other callers). But
    if it derails things, it may not be worth doing now. I can live
    without it.

-Peff