@matheusbsbs / matheus-bagliano / commits / eab4ac6a23

ls-files: fix a trivial dir_clear() leak

Fix an edge case that was missed when the dir_clear() call was added in eceba532141 (dir: fix problematic API to avoid memory leaks, 2020-08-18), we need to also clean up when we're about to exit with non-zero. That commit says, on the topic of the dir_clear() API and UNLEAK(): [...]two of them clearly thought about leaks since they had an UNLEAK(dir) directive, which to me suggests that the method to free the data was too unclear. I think that 0e5bba53af7 (add UNLEAK annotation for reducing leak false positives, 2017-09-08) which added the UNLEAK() makes it clear that that wasn't the case, rather it was the desire to avoid the complexity of freeing the memory at the end of the program. This does add a bit of complexity, but I think it's worth it to just fix these leaks when it's easy in built-ins. It allows them to serve as canaries for underlying APIs that shouldn't be leaking, it encourages us to make those freeing APIs nicer for all their users, and it prevents other leaking regressions by being able to mark the entire test as TEST_PASSES_SANITIZE_LEAK=true. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Oct 7, 2021 at 12:01 UTC eab4ac6a233e505e78fc8b04df03d58628d5ff3e

5 files changed +10 -8

builtin/ls-files.c

+5 -8

@@ -672,6 +672,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)

672 N_("suppress duplicate entries")),

673 OPT_END()

674 };

675 + int ret = 0;

676

677 if (argc == 2 && !strcmp(argv[1], "-h"))

678 usage_with_options(ls_files_usage, builtin_ls_files_options);

@@ -775,16 +776,12 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)

776 if (show_resolve_undo)

777 show_ru_info(the_repository->index);

778

778 - if (ps_matched) {

779 - int bad;

780 - bad = report_path_error(ps_matched, &pathspec);

781 - if (bad)

782 - fprintf(stderr, "Did you forget to 'git add'?\n");

783 -

784 - return bad ? 1 : 0;

779 + if (ps_matched && report_path_error(ps_matched, &pathspec)) {

780 + fprintf(stderr, "Did you forget to 'git add'?\n");

781 + ret = 1;

782 }

783

784 dir_clear(&dir);

785 free(max_prefix);

789 - return 0;

786 + return ret;

787 }

t/t3005-ls-files-relative.sh

@@ -5,6 +5,7 @@ test_description='ls-files tests with relative paths

5 This test runs git ls-files with various relative path arguments.

6 '

8 +TEST_PASSES_SANITIZE_LEAK=true

9 . ./test-lib.sh

11 test_expect_success 'prepare' '

t/t3020-ls-files-error-unmatch.sh

@@ -9,6 +9,8 @@ This test runs git ls-files --error-unmatch to ensure it correctly

9 returns an error when a non-existent path is provided on the command

10 line.

11 '

12 +

13 +TEST_PASSES_SANITIZE_LEAK=true

14 . ./test-lib.sh

16 test_expect_success 'setup' '

t/t3700-add.sh

@@ -5,6 +5,7 @@

6 test_description='Test of git add, including the -- option.'

8 +TEST_PASSES_SANITIZE_LEAK=true

9 . ./test-lib.sh

11 # Test the file mode "$1" of the file "$2" in the index.

t/t7104-reset-hard.sh

@@ -2,6 +2,7 @@

3 test_description='reset --hard unmerged'

5 +TEST_PASSES_SANITIZE_LEAK=true

6 . ./test-lib.sh

8 test_expect_success setup '