On Thu, May 30, 2024 at 02:44:22AM -0400, Jeff King wrote: > Commit b6f51e3db9 (mv: cleanup empty WORKING_DIRECTORY, 2022-08-09) > added an auxiliary array where we store directory arguments that we see > while processing the incoming arguments. After actually moving things, > we then use that array to remove now-empty directories, and then > immediately free the array. > > 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. > > Note that the adjacent "a_src_dir" strbuf does not suffer the same > problem; it is only allocated during the removal step. > > Signed-off-by: Jeff King <peff@xxxxxxxx> > --- > Reported as "new" by Coverity, but I think that is only because of the > "goto out". Before your recent patches it was a straight "return", which > was even worse. ;) Ouf of curiosity, did you check whether this makes any additional tests pass with SANITIZE=leak? The fix itself looks good to me, thanks. Patrick
Attachment:
signature.asc
Description: PGP signature
