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

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

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.