Re: [PATCH 3/5] mv: move src_dir cleanup to end of cmd_mv()
- From
Patrick Steinhardt <ps@pks.im>
- Date
- May 30, 2024, 07:24 UTC
- Message-ID
- <ZlgpsrKN7N38V2GP@tanuki>
- In-Reply-To
- <20240530072159.GF1949834@coredump.intra.peff.net>
On Thu, May 30, 2024 at 03:21:59AM -0400, Jeff King wrote:
Show 24 quoted lines
> 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