Subject: Re: [PATCH v2 00/11] built-ins: fix common memory leaks

From: Junio C Hamano <[email-masked]>
Date: Fri, 01 Jul 2022 11:58:52 -0700

Ævar Arnfjörð Bjarmason  <[email-masked]> writes:

>  * Replace 8/11, maybe that solution is going overboard, we could also
>    just drop it from this series...

The "solution" part is to forbid merge_working_tree() to use
anything that relies on the topts before it bails out due to
unmerged_cache(), that could be done with only a minor reordering of
the logic.

And I think that is a reasonable thing to do---if we are looking at
an unmerged index, we would not want to run unpack-trees on it at
all, so setting up the topts structure does not make much sense
before we fully know we cannot bail out early.

On the other hand, it does make the patch go near overboard to
extract the topt initialization logic for this single user into a
static helper function.  If we later find out that my "that is a
reasonable thing to do" above is not true, or some setting of topts
members need to become conditional, we would probably need to undo
that part of this patch.

The latter "going overboard" part is the majority of change.  We
definitely should *not* discard the essence of this step to stop
leaking, but should think about stopping at the smaller change,
perhaps?

Having said that, all the other patches in the series looked good.

Thanks, will queue.