From a2fb7672c03eef86b8dbf33699c12307d504a299 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:39 +0200 Subject: [PATCH 1/6] 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(-) 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; } From 96c101257b00b73f0185fc6630160a5f0b5d4277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:40 +0200 Subject: [PATCH 2/6] 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(+) 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; } From b202e51b15401207667261f2cb384e6faa6ed5c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:41 +0200 Subject: [PATCH 3/6] 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(-) 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 From 27ff1fbc5dd5ca5f03a65bfc734c913703e8cdf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:42 +0200 Subject: [PATCH 4/6] clone: fix a memory leak of the "git_dir" variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At this point in cmd_clone the "git_dir" is always either an xstrdup()'d string, or something we got from mkpathdup(). Let's free() it before we clobber it. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/clone.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/builtin/clone.c b/builtin/clone.c index 559acf9e03..fb377b2765 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -1040,8 +1040,10 @@ int cmd_clone(int argc, const char **argv, const char *prefix) init_db(git_dir, real_git_dir, option_template, GIT_HASH_UNKNOWN, NULL, INIT_DB_QUIET); - if (real_git_dir) + if (real_git_dir) { + free((char *)git_dir); git_dir = real_git_dir; + } /* * additional config can be injected with -c, make sure it's included From c270b055d94b643b0ac8bcb390b82330aee01a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:43 +0200 Subject: [PATCH 5/6] submodule--helper: fix small memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a missing strbuf_release() and a clear_pathspec() to the submodule--helper. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6298cbdd4e..a157656a48 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -3220,6 +3220,7 @@ static void die_on_index_match(const char *path, int force) } free(ps_matched); } + clear_pathspec(&ps); } static void die_on_repo_without_commits(const char *path) @@ -3231,6 +3232,7 @@ static void die_on_repo_without_commits(const char *path) if (resolve_gitlink_ref(path, "HEAD", &oid) < 0) die(_("'%s' does not have a commit checked out"), path); } + strbuf_release(&sb); } static int module_add(int argc, const char **argv, const char *prefix) From 3c8150497f3c77a2c57e73c53bd3de933b85d9a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Fri, 22 Oct 2021 10:55:44 +0200 Subject: [PATCH 6/6] reflog: free() ref given to us by dwim_log() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When dwim_log() returns the "ref" is always ether NULL or an xstrdup()'d string. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/reflog.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/reflog.c b/builtin/reflog.c index bd4c669918..175c83e7cc 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -653,6 +653,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) should_expire_reflog_ent, reflog_expiry_cleanup, &cb); + free(ref); } return status; }