Subject: Re: [PATCH 07/10] unpack-trees API: rename clear_unpack_trees_porcelain()

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 04 Oct 2021 13:10:37 +0200

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 <[email-masked]>
>> ---
>>   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).