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. ;) builtin/mv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/mv.c b/builtin/mv.c index 81ca910de6..852b4e92c1 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -556,7 +556,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } strbuf_release(&a_src_dir); - free(src_dir); if (dirty_paths.nr) advise_on_moving_dirty_path(&dirty_paths); @@ -571,6 +570,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) ret = 0; out: + free(src_dir); free(dst_w_slash); string_list_clear(&src_for_dst, 0); string_list_clear(&dirty_paths, 0); -- 2.45.1.692.gbe047d9c60
