Re: [PATCH v4 3/8] rebase --merge: fix reflog when continuing

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Oct 25, 2022, 10:08 UTC
Message-ID: <86699708-d631-fb49-482c-af27204a3570@dunelm.org.uk>
In-Reply-To: <xmqqk04tm62o.fsf@gitster.g>

Hi Junio

On 21/10/2022 18:37, Junio C Hamano wrote:

Show 8 quoted lines

> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> ... This
>> introduces a memory leak similar to the one leaking GIT_REFLOG_ACTION
>> in pick_commits().
> 
> Is it just the matter of freeing previous_reflog_action after you
> call setenv(), or does it take much more involved changes?

We should be freeing previous_reflog_action - I had misremembered a previous discussion about setenv() but the manual page makes it quite clear that it copies the strings passed to it. However we call setenv() each time we pick a commit and that leaks the previous value. The solution is to avoid calling setenv() at all and instead use the env member of struct child_process when we run "git commit".

Show 6 quoted lines

>> Both of these will be fixed in a future series that
>> stops the sequencer calling setenv().
> 
> If it gets fixed in a future step in the same series, that is a
> different matter, but if it is easy enough not to deliberately
> introduce a new leak, we'd prefer to do so.

It's a couple of patches to fix which are more or less finished, I'm planning to send them once this series is in next.

Best Wishes

Phillip