Subject: Re: [PATCH 3/5] mv: move src_dir cleanup to end of cmd_mv()

From: Patrick Steinhardt <[email-masked]>
Date: Thu, 30 May 2024 09:24:34 +0200

On Thu, May 30, 2024 at 03:21:59AM -0400, Jeff King wrote:
> On Thu, May 30, 2024 at 09:04:28AM +0200, Patrick Steinhardt wrote:
> 
> > > But if the actual move queues any errors in only_match_skip_worktree,
> > > that can cause us to jump straight to the "out" label to clean up,
> > > skipping the free() and leaking the array.
> > > 
> > > Let's push the free() down past the "out" label so that we always clean
> > > up (the array is initialized to NULL, so this is always safe). We'll
> > > hold on to the memory a little longer than necessary, but clarity is
> > > more important than micro-optimizing here.
> > [...]
> > 
> > Ouf of curiosity, did you check whether this makes any additional tests
> > pass with SANITIZE=leak?
> 
> No, I didn't. I think you can only trigger it with a sparse index, at
> which point it seemed like diminishing returns to try to reproduce.
> 
> But running in "check" mode is not too hard...
> 
> ...time passes...
> 
> Looks like no. The obvious candidate would be t7002-mv-sparse-checkout,
> but it looks like the sparse-checkout code has minor leaks itself.

Okay, thanks for double checking! I was mostly asking because I plan to
send another leak fixes series to the mailing list later this week.

Patrick