Skip to content

Commit eab4ac6

Browse files
avargitster
avar
authored and
gitster
committed
ls-files: fix a trivial dir_clear() leak
Fix an edge case that was missed when the dir_clear() call was added in eceba53 (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 0e5bba5 (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 6ad66ab commit eab4ac6

5 files changed

Lines changed: 10 additions & 8 deletions

File tree

‎builtin/ls-files.c‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -672,6 +672,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
672672
N_("suppress duplicate entries")),
673673
OPT_END()
674674
};
675+
int ret = 0;
675676

676677
if (argc == 2 && !strcmp(argv[1], "-h"))
677678
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)
775776
if (show_resolve_undo)
776777
show_ru_info(the_repository->index);
777778

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;
785782
}
786783

787784
dir_clear(&dir);
788785
free(max_prefix);
789-
return 0;
786+
return ret;
790787
}

‎t/t3005-ls-files-relative.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ test_description='ls-files tests with relative paths
55
This test runs git ls-files with various relative path arguments.
66
'
77

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

1011
test_expect_success 'prepare' '

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ This test runs git ls-files --error-unmatch to ensure it correctly
99
returns an error when a non-existent path is provided on the command
1010
line.
1111
'
12+
13+
TEST_PASSES_SANITIZE_LEAK=true
1214
. ./test-lib.sh
1315

1416
test_expect_success 'setup' '

‎t/t3700-add.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

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

8+
TEST_PASSES_SANITIZE_LEAK=true
89
. ./test-lib.sh
910

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

‎t/t7104-reset-hard.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='reset --hard unmerged'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
test_expect_success setup '

0 commit comments

Comments
 (0)