From a2fb7672c03eef86b8dbf33699c12307d504a299 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Fri, 22 Oct 2021 10:55:39 +0200 Subject: grep: prefer "struct grep_opt" over its "void *" equivalent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stylistically fix up code added in bfac23d9534 (grep: Fix two memory leaks, 2010-01-30). We usually don't use the "arg" at all once we've casted it to the struct we want, let's not do that here when we're freeing it. Perhaps it was thought that a cast to "void *" would otherwise be needed? Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/grep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'builtin/grep.c') diff --git a/builtin/grep.c b/builtin/grep.c index 8af5249a7b..fd184c182a 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -199,8 +199,8 @@ static void *run(void *arg) grep_source_clear_data(&w->source); work_done(w); } - free_grep_patterns(arg); - free(arg); + free_grep_patterns(opt); + free(opt); return (void*) (intptr_t) hit; } -- cgit 1.2.3-korg From 96c101257b00b73f0185fc6630160a5f0b5d4277 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Fri, 22 Oct 2021 10:55:40 +0200 Subject: grep: use object_array_clear() in cmd_grep() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free the "struct object_array" before exiting. This makes grep tests (e.g. "t7815-grep-binary.sh") a bit happer under SANITIZE=leak. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/grep.c | 1 + 1 file changed, 1 insertion(+) (limited to 'builtin/grep.c') diff --git a/builtin/grep.c b/builtin/grep.c index fd184c182a..555b2ab600 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -1196,6 +1196,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) run_pager(&opt, prefix); clear_pathspec(&pathspec); free_grep_patterns(&opt); + object_array_clear(&list); free_repos(); return !hit; } -- cgit 1.2.3-korg From b202e51b15401207667261f2cb384e6faa6ed5c3 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Fri, 22 Oct 2021 10:55:41 +0200 Subject: grep: fix a "path_list" memory leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Free the "path_list" used in builtin/grep.c, it was declared as STRING_LIST_INIT_NODUP, let's change it to a STRING_LIST_INIT_DUP since an early user in cmd_grep() appends a string passed via parse-options.c to it, which needs to be duplicated. Let's then convert the remaining callers to use string_list_append_nodup() instead, allowing us to free the list. This makes all the tests in t7811-grep-open.sh pass, 6/10 would fail before this change. The only remaining failure would have been due to a stray "git checkout" (which still leaks memory). In this case we can use a "git reset --hard" instead, so let's do that, and move the test_when_finished() above the code that would modify the relevant file. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/grep.c | 9 +++++---- t/t7811-grep-open.sh | 3 ++- 2 files changed, 7 insertions(+), 5 deletions(-) (limited to 'builtin/grep.c') diff --git a/builtin/grep.c b/builtin/grep.c index 555b2ab600..9e34a820ad 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -401,7 +401,7 @@ static void append_path(struct grep_opt *opt, const void *data, size_t len) if (len == 1 && *(const char *)data == '\0') return; - string_list_append(path_list, xstrndup(data, len)); + string_list_append_nodup(path_list, xstrndup(data, len)); } static void run_pager(struct grep_opt *opt, const char *prefix) @@ -839,7 +839,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) struct grep_opt opt; struct object_array list = OBJECT_ARRAY_INIT; struct pathspec pathspec; - struct string_list path_list = STRING_LIST_INIT_NODUP; + struct string_list path_list = STRING_LIST_INIT_DUP; int i; int dummy; int use_index = 1; @@ -1159,8 +1159,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix) strbuf_addf(&buf, "+/%s%s", strcmp("less", pager) ? "" : "*", opt.pattern_list->pattern); - string_list_append(&path_list, - strbuf_detach(&buf, NULL)); + string_list_append_nodup(&path_list, + strbuf_detach(&buf, NULL)); } } @@ -1195,6 +1195,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) if (hit && show_in_pager) run_pager(&opt, prefix); clear_pathspec(&pathspec); + string_list_clear(&path_list, 0); free_grep_patterns(&opt); object_array_clear(&list); free_repos(); diff --git a/t/t7811-grep-open.sh b/t/t7811-grep-open.sh index a98785da79..1dd07141a7 100755 --- a/t/t7811-grep-open.sh +++ b/t/t7811-grep-open.sh @@ -3,6 +3,7 @@ test_description='git grep --open-files-in-pager ' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-pager.sh unset PAGER GIT_PAGER @@ -114,8 +115,8 @@ test_expect_success 'modified file' ' unrelated EOF + test_when_finished "git reset --hard" && echo "enum grep_pat_token" >unrelated && - test_when_finished "git checkout HEAD unrelated" && GIT_PAGER=./less git grep -F -O "enum grep_pat_token" >out && test_cmp expect actual && test_must_be_empty out -- cgit 1.2.3-korg