All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: phillip.wood@dunelm.org.uk
Cc: git@vger.kernel.org, "Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Andrzej Hunt" <ajrhunt@google.com>, "Jeff King" <peff@peff.net>
Subject: Re: [PATCH 07/10] unpack-trees API: rename clear_unpack_trees_porcelain()
Date: Mon, 04 Oct 2021 13:10:37 +0200	[thread overview]
Message-ID: <87fsthnix1.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <ebc72146-4431-c29c-ebab-37cf886cf28f@gmail.com>


On Mon, Oct 04 2021, Phillip Wood wrote:

> Hi Ævar
>
> On 04/10/2021 01:46, Ævar Arnfjörð Bjarmason wrote:
>> Since a preceding commit we've been using
>> clear_unpack_trees_porcelain() to call dir_clear(). So it's no longer
>> a function that corresponds to setup_unpack_trees_porcelain(), as it
>> was when it was added in 1c41d2805e4 (unpack_trees_options: free
>> messages when done, 2018-05-21).
>> Instead it's similar to strbuf_release() and other similar generic
>> "free" functions. Let's rename it to avoid any future confusion on the
>> topic.
>> Let's also update the API documentation for it to note this, and to
>> cover e.g. the code added around update_sparsity() in
>> 4ee5d50fc39 (sparse-checkout: use improved unpack_trees porcelain
>> messages, 2020-03-27).
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>>   builtin/checkout.c        | 2 +-
>>   builtin/sparse-checkout.c | 2 +-
>>   merge-ort.c               | 2 +-
>>   merge-recursive.c         | 2 +-
>>   merge.c                   | 4 ++--
>>   reset.c                   | 2 +-
>
> I was expecting to see sequencer.c here, but it appears it does not
> call clear_unpack_trees_porcelain() to free the memory allocated with 
> setup_unpack_trees_porcelain(). If you're interested in fixing memory
> leaks from unpack trees it might be worth checking that all the calls
> to setup_unpack_trees_porcelain() have a matching call to 
> clear_unpack_trees_porcelain().

Well spotted, I thought I was doing that, but see that I missed this one
special-case, will fix.

With this series grepping for "UNPACK_TREES_OPTIONS_INIT" is better (and
knowing about the one special-case of merge-recursive.c).

  reply	other threads:[~2021-10-04 11:11 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-04  0:46 [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 01/10] unpack-trees.[ch]: define and use a UNPACK_TREES_OPTIONS_INIT Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 02/10] merge-recursive.c: call a new unpack_trees_options_init() function Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:41     ` Ævar Arnfjörð Bjarmason
2021-10-04 15:04       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 03/10] unpack-trees.[ch]: embed "dir" in "struct unpack_trees_options" Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 04/10] unpack-trees API: don't have clear_unpack_trees_porcelain() reset Ævar Arnfjörð Bjarmason
2021-10-04  9:31   ` Phillip Wood
2021-10-04 11:12     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 15:20     ` Ævar Arnfjörð Bjarmason
2021-10-04 16:28       ` Elijah Newren
2021-10-04  0:46 ` [PATCH 05/10] dir.[ch]: make DIR_INIT mandatory Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 06/10] dir.c: get rid of lazy initialization Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 07/10] unpack-trees API: rename clear_unpack_trees_porcelain() Ævar Arnfjörð Bjarmason
2021-10-04  9:38   ` Phillip Wood
2021-10-04 11:10     ` Ævar Arnfjörð Bjarmason [this message]
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 08/10] unpack-trees: don't leak memory in verify_clean_subdirectory() Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04  0:46 ` [PATCH 09/10] merge.c: avoid duplicate unpack_trees_options_release() code Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:50     ` Ævar Arnfjörð Bjarmason
2021-10-04  0:46 ` [PATCH 10/10] built-ins: plug memory leaks with unpack_trees_options_release() Ævar Arnfjörð Bjarmason
2021-10-04 13:45   ` Elijah Newren
2021-10-04 14:54     ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45 ` [PATCH 00/10] unpack-trees & dir APIs: fix memory leaks Elijah Newren

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=87fsthnix1.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=ajrhunt@google.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=phillip.wood@dunelm.org.uk \
    /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.