git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Taylor Blau" <me@ttaylorr.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 3/6] grep: fix a "path_list" memory leak
Date: Fri, 22 Oct 2021 10:55:41 +0200	[thread overview]
Message-ID: <patch-v2-3.6-8e941e40711-20211022T085306Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v2-0.6-00000000000-20211022T085306Z-avarab@gmail.com>

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 <avarab@gmail.com>
---
 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 555b2ab6008..9e34a820ad4 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 a98785da795..1dd07141a7d 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
-- 
2.33.1.1494.g88b39a443e1


  parent reply	other threads:[~2021-10-22  8:56 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-21 15:57 [PATCH 0/6] leaks: miscellaneous small leak fixes Ævar Arnfjörð Bjarmason
2021-10-21 15:57 ` [PATCH 1/6] grep: prefer "struct grep_opt" over its "void *" Ævar Arnfjörð Bjarmason
2021-10-21 23:52   ` Junio C Hamano
2021-10-21 15:57 ` [PATCH 2/6] grep: use object_array_clear() in cmd_grep() Ævar Arnfjörð Bjarmason
2021-10-21 23:56   ` Junio C Hamano
2021-10-21 23:58   ` Junio C Hamano
2021-10-21 15:57 ` [PATCH 3/6] clone: fix a memory leak of the "git_dir" variable Ævar Arnfjörð Bjarmason
2021-10-22  0:07   ` Junio C Hamano
2021-10-21 15:57 ` [PATCH 4/6] submodule--helper: fix small memory leaks Ævar Arnfjörð Bjarmason
2021-10-21 15:57 ` [PATCH 5/6] reflog: free() ref given to us by dwim_log() Ævar Arnfjörð Bjarmason
2021-10-22  0:16   ` Junio C Hamano
2021-10-21 15:57 ` [PATCH 6/6] repack: stop leaking a "struct child_process" Ævar Arnfjörð Bjarmason
2021-10-22  0:22   ` Junio C Hamano
2021-10-22  8:55 ` [PATCH v2 0/6] leaks: miscellaneous small leak fixes Ævar Arnfjörð Bjarmason
2021-10-22  8:55   ` [PATCH v2 1/6] grep: prefer "struct grep_opt" over its "void *" equivalent Ævar Arnfjörð Bjarmason
2021-10-22  8:55   ` [PATCH v2 2/6] grep: use object_array_clear() in cmd_grep() Ævar Arnfjörð Bjarmason
2021-10-22  8:55   ` Ævar Arnfjörð Bjarmason [this message]
2021-10-22  8:55   ` [PATCH v2 4/6] clone: fix a memory leak of the "git_dir" variable Ævar Arnfjörð Bjarmason
2021-10-22  8:55   ` [PATCH v2 5/6] submodule--helper: fix small memory leaks Ævar Arnfjörð Bjarmason
2021-10-22  8:55   ` [PATCH v2 6/6] reflog: free() ref given to us by dwim_log() Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v2-3.6-8e941e40711-20211022T085306Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).