From: Andrzej Hunt <andrzej@ahunt.org>
To: Elijah Newren <newren@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Christian Couder <christian.couder@gmail.com>
Subject: Re: [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 2
Date: Sun, 25 Jul 2021 15:05:31 +0200 [thread overview]
Message-ID: <e83899bb-3d49-68b2-b67b-76ab824adfc3@ahunt.org> (raw)
In-Reply-To: <CABPp-BG0a0OM7s7cmO8yCeyA5TOCD_yOSJJepQE8MFEHct4EQA@mail.gmail.com>
On 21/06/2021 23:54, Elijah Newren wrote:
> On Sun, Jun 20, 2021 at 8:14 AM <andrzej@ahunt.org> wrote:
>>
>> From: Andrzej Hunt <andrzej@ahunt.org>
>>
>> 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
next prev parent reply other threads:[~2021-07-25 13:05 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-20 15:11 [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 2 andrzej
2021-06-20 15:11 ` [PATCH 01/12] fmt-merge-msg: free newly allocated temporary strings when done andrzej
2021-06-21 20:34 ` Elijah Newren
2021-06-20 15:11 ` [PATCH 02/12] environment: move strbuf into block to plug leak andrzej
2021-06-21 20:49 ` Elijah Newren
2021-06-26 8:27 ` René Scharfe
2021-06-20 15:11 ` [PATCH 03/12] builtin/submodule--helper: release unused strbuf to avoid leak andrzej
2021-06-20 15:11 ` [PATCH 04/12] builtin/for-each-repo: remove unnecessary argv copy to plug leak andrzej
2021-06-21 20:55 ` Elijah Newren
2021-06-20 15:11 ` [PATCH 05/12] diffcore-rename: move old_dir/new_dir definition " andrzej
2021-06-21 14:01 ` Elijah Newren
2021-06-20 15:11 ` [PATCH 06/12] ref-filter: also free head for ATOM_HEAD to avoid leak andrzej
2021-06-21 21:10 ` Elijah Newren
2021-06-20 15:11 ` [PATCH 07/12] read-cache: call diff_setup_done " andrzej
2021-06-21 21:17 ` Elijah Newren
2021-06-20 15:12 ` [PATCH 08/12] convert: release strbuf " andrzej
2021-06-21 20:31 ` Elijah Newren
2021-06-20 15:12 ` [PATCH 09/12] builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv andrzej
2021-06-20 15:12 ` [PATCH 10/12] builtin/merge: free found_ref when done andrzej
2021-06-21 21:27 ` Elijah Newren
2021-06-20 15:12 ` [PATCH 11/12] builtin/rebase: fix options.strategy memory lifecycle andrzej
2021-06-20 18:14 ` Phillip Wood
2021-06-21 21:39 ` Elijah Newren
2021-06-22 9:02 ` Phillip Wood
2021-07-25 13:03 ` Andrzej Hunt
2021-07-27 19:34 ` Phillip Wood
2021-06-20 15:12 ` [PATCH 12/12] reset: clear_unpack_trees_porcelain to plug leak andrzej
2021-06-21 21:44 ` Elijah Newren
2021-06-21 21:54 ` [PATCH 00/12] Fix all leaks in tests t0002-t0099: Part 2 Elijah Newren
2021-07-25 13:05 ` Andrzej Hunt [this message]
2021-07-26 8:01 ` Christian Couder
2021-07-25 13:08 ` [PATCH v2 " andrzej
2021-07-25 13:08 ` [PATCH v2 01/12] fmt-merge-msg: free newly allocated temporary strings when done andrzej
2021-07-26 19:20 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 02/12] environment: move strbuf into block to plug leak andrzej
2021-07-25 13:08 ` [PATCH v2 03/12] builtin/submodule--helper: release unused strbuf to avoid leak andrzej
2021-07-25 13:08 ` [PATCH v2 04/12] builtin/for-each-repo: remove unnecessary argv copy to plug leak andrzej
2021-07-26 20:02 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 05/12] diffcore-rename: move old_dir/new_dir definition " andrzej
2021-07-26 20:02 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 06/12] ref-filter: also free head for ATOM_HEAD to avoid leak andrzej
2021-07-26 20:04 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 07/12] read-cache: call diff_setup_done " andrzej
2021-07-26 20:10 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 08/12] convert: release strbuf " andrzej
2021-07-26 20:15 ` Junio C Hamano
2021-07-25 13:08 ` [PATCH v2 09/12] builtin/mv: free or UNLEAK multiple pointers at end of cmd_mv andrzej
2021-07-25 13:08 ` [PATCH v2 10/12] builtin/merge: free found_ref when done andrzej
2021-07-25 13:08 ` [PATCH v2 11/12] builtin/rebase: fix options.strategy memory lifecycle andrzej
2021-07-25 13:08 ` [PATCH v2 12/12] reset: clear_unpack_trees_porcelain to plug leak andrzej
2021-07-26 20:20 ` [PATCH v2 00/12] Fix all leaks in tests t0002-t0099: Part 2 Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e83899bb-3d49-68b2-b67b-76ab824adfc3@ahunt.org \
--to=andrzej@ahunt.org \
--cc=christian.couder@gmail.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.