[PATCH 0/9] test-tool: fix memory leaks

[PATCH 0/9] test-tool: fix memory leaks

[Ævar Arnfjörð Bjarmason]

This is a series of trivial leak-fixes which allows us to mark various
tests as entirely passing under SANITIZE=leak, the reasn they weren't
passing before was because we'd spot memory leaks in test-tool, rather
than git itself.

This is a sibling series to my just-submitted series for fixing memory
leaks in built-ins[1], but the the two do not textually or
semantically conflict.

There are still a few other test-tool memory leaks, but these are the
ones I had patches ready for, having focused on trying to get t00*.sh
passing first, and then picking some low-hanging fruit after that.

Passing CI for this series can be found at [2].

1. https://lore.kernel.org/git/cover-00.11-00000000000-20220630T175714Z-avarab@gmail.com/
2. https://github.com/avar/git/tree/avar/test-tool-memory-leaks

Ævar Arnfjörð Bjarmason (9):
  test-tool test-hash: fix a memory leak
  test-tool path-utils: fix a memory leak
  test-tool {dump,scrap}-cache-tree: fix memory leaks
  test-tool urlmatch-normalization: fix a memory leak
  test-tool regex: call regfree(), fix memory leaks
  test-tool json-writer: fix memory leaks
  test-tool bloom: fix a memory leak
  test-tool ref-store: fix a memory leak
  test-tool delta: fix a memory leak

 t/helper/test-bloom.c                  |  2 ++
 t/helper/test-delta.c                  | 21 +++++++++-----
 t/helper/test-dump-cache-tree.c        |  7 ++++-
 t/helper/test-hash.c                   |  1 +
 t/helper/test-json-writer.c            | 16 ++++++++---
 t/helper/test-path-utils.c             | 10 +++++--
 t/helper/test-ref-store.c              |  1 +
 t/helper/test-regex.c                  | 40 ++++++++++++++++----------
 t/helper/test-scrap-cache-tree.c       |  1 +
 t/helper/test-urlmatch-normalization.c | 11 +++++--
 t/t0015-hash.sh                        |  3 +-
 t/t0019-json-writer.sh                 |  2 ++
 t/t0060-path-utils.sh                  |  1 +
 t/t0090-cache-tree.sh                  |  2 ++
 t/t0095-bloom.sh                       |  2 +-
 t/t0110-urlmatch-normalization.sh      |  2 ++
 t/t5303-pack-corruption-resilience.sh  |  2 ++
 t/t5308-pack-detect-duplicates.sh      |  2 ++
 t/t5309-pack-delta-cycles.sh           |  2 ++
 t/t5321-pack-large-objects.sh          |  2 ++
 t/t7812-grep-icase-non-ascii.sh        |  1 +
 21 files changed, 97 insertions(+), 34 deletions(-)

-- 
2.37.0.874.g7d3439f13c4

[PATCH 1/9] test-tool test-hash: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool test-hash" which has been there since
b57cbbf8a86 (test-sha1: test hashing large buffer, 2006-06-24), as a
result we can mark more tests as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-hash.c              | 1 +
 t/t0015-hash.sh                   | 3 ++-
 t/t5308-pack-detect-duplicates.sh | 2 ++
 t/t5309-pack-delta-cycles.sh      | 2 ++
 t/t5321-pack-large-objects.sh     | 2 ++
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-hash.c b/t/helper/test-hash.c
index 261c545b9d1..5860dab0ffa 100644
--- a/t/helper/test-hash.c
+++ b/t/helper/test-hash.c
@@ -54,5 +54,6 @@ int cmd_hash_impl(int ac, const char **av, int algo)
 		fwrite(hash, 1, algop->rawsz, stdout);
 	else
 		puts(hash_to_hex_algop(hash, algop));
+	free(buffer);
 	return 0;
 }
diff --git a/t/t0015-hash.sh b/t/t0015-hash.sh
index 086822fc45b..0a087a1983d 100755
--- a/t/t0015-hash.sh
+++ b/t/t0015-hash.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
 test_description='test basic hash implementation'
-. ./test-lib.sh
 
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
 
 test_expect_success 'test basic SHA-1 hash values' '
 	test-tool sha1 </dev/null >actual &&
diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
index 693b2411c89..655cafa0541 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='handling of duplicate objects in incoming packfiles'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
index 55b787630fc..4e910c5b9d2 100755
--- a/t/t5309-pack-delta-cycles.sh
+++ b/t/t5309-pack-delta-cycles.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test index-pack handling of delta cycles in packfiles'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
diff --git a/t/t5321-pack-large-objects.sh b/t/t5321-pack-large-objects.sh
index 8a56d98a0e8..70770fe274d 100755
--- a/t/t5321-pack-large-objects.sh
+++ b/t/t5321-pack-large-objects.sh
@@ -6,6 +6,8 @@
 test_description='git pack-object with "large" deltas
 
 '
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
-- 
2.37.0.874.g7d3439f13c4

[PATCH 2/9] test-tool path-utils: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool path-utils", as a result we can mark
the corresponding test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-path-utils.c | 10 ++++++++--
 t/t0060-path-utils.sh      |  1 +
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 229ed416b0e..380437cfe02 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -294,11 +294,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
 int cmd__path_utils(int argc, const char **argv)
 {
 	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
-		char *buf = xmallocz(strlen(argv[2]));
+		char *to_free = NULL;
+		char *buf = to_free = xmallocz(strlen(argv[2]));
 		int rv = normalize_path_copy(buf, argv[2]);
 		if (rv)
 			buf = "++failed++";
 		puts(buf);
+		free(to_free);
 		return 0;
 	}
 
@@ -356,7 +358,10 @@ int cmd__path_utils(int argc, const char **argv)
 		int nongit_ok;
 		setup_git_directory_gently(&nongit_ok);
 		while (argc > 3) {
-			puts(prefix_path(prefix, prefix_len, argv[3]));
+			char *pfx = prefix_path(prefix, prefix_len, argv[3]);
+
+			puts(pfx);
+			free(pfx);
 			argc--;
 			argv++;
 		}
@@ -366,6 +371,7 @@ int cmd__path_utils(int argc, const char **argv)
 	if (argc == 4 && !strcmp(argv[1], "strip_path_suffix")) {
 		char *prefix = strip_path_suffix(argv[2], argv[3]);
 		printf("%s\n", prefix ? prefix : "(null)");
+		free(prefix);
 		return 0;
 	}
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index aa35350b6f3..1f2007e62b7 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -5,6 +5,7 @@
 
 test_description='Test various path utilities'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 norm_path() {
-- 
2.37.0.874.g7d3439f13c4

Re: [PATCH 2/9] test-tool path-utils: fix a memory leak

[Eric Sunshine]

On Thu, Jun 30, 2022 at 7:51 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Fix a memory leak in "test-tool path-utils", as a result we can mark
> the corresponding test as passing with SANITIZE=leak using
> "TEST_PASSES_SANITIZE_LEAK=true".
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
> @@ -294,11 +294,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
>  int cmd__path_utils(int argc, const char **argv)
>  {
>         if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
> -               char *buf = xmallocz(strlen(argv[2]));
> +               char *to_free = NULL;
> +               char *buf = to_free = xmallocz(strlen(argv[2]));

Is there a non-obvious reason that `to_free` is initialized to NULL
before being immediately overwritten with the result of xmallocz()?

Also, pure nit, but it may be a bit more idiomatic (though I could be
wrong) written as:

    char *buf, *to_free;
    buf = to_free = xmallocz(...);

Re: [PATCH 2/9] test-tool path-utils: fix a memory leak

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 01 2022, Eric Sunshine wrote:

> On Thu, Jun 30, 2022 at 7:51 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> Fix a memory leak in "test-tool path-utils", as a result we can mark
>> the corresponding test as passing with SANITIZE=leak using
>> "TEST_PASSES_SANITIZE_LEAK=true".
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>> ---
>> diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
>> @@ -294,11 +294,13 @@ static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
>>  int cmd__path_utils(int argc, const char **argv)
>>  {
>>         if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
>> -               char *buf = xmallocz(strlen(argv[2]));
>> +               char *to_free = NULL;
>> +               char *buf = to_free = xmallocz(strlen(argv[2]));
>
> Is there a non-obvious reason that `to_free` is initialized to NULL
> before being immediately overwritten with the result of xmallocz()?
>
> Also, pure nit, but it may be a bit more idiomatic (though I could be
> wrong) written as:
>
>     char *buf, *to_free;
>     buf = to_free = xmallocz(...);

Thanks, I'll change it to be like that. I wouldn't put "= NULL" there
normally, that part snuck in from some earlier version, and escaped my
own pre-review.

[PATCH 3/9] test-tool {dump,scrap}-cache-tree: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks in two test-tools used by t0090-cache-tree.sh. As a
result we can mark the test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-dump-cache-tree.c  | 7 ++++++-
 t/helper/test-scrap-cache-tree.c | 1 +
 t/t0090-cache-tree.sh            | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c
index 6a3f88f5f5d..0d6d7f1ecbf 100644
--- a/t/helper/test-dump-cache-tree.c
+++ b/t/helper/test-dump-cache-tree.c
@@ -59,11 +59,16 @@ int cmd__dump_cache_tree(int ac, const char **av)
 {
 	struct index_state istate;
 	struct cache_tree *another = cache_tree();
+	int ret;
+
 	setup_git_directory();
 	if (read_cache() < 0)
 		die("unable to read index file");
 	istate = the_index;
 	istate.cache_tree = another;
 	cache_tree_update(&istate, WRITE_TREE_DRY_RUN);
-	return dump_cache_tree(active_cache_tree, another, "");
+	ret = dump_cache_tree(active_cache_tree, another, "");
+	cache_tree_free(&another);
+
+	return ret;
 }
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index 393f1604ff9..026c802479d 100644
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -12,6 +12,7 @@ int cmd__scrap_cache_tree(int ac, const char **av)
 	hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
 	if (read_cache() < 0)
 		die("unable to read index file");
+	cache_tree_free(&active_cache_tree);
 	active_cache_tree = NULL;
 	if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
 		die("unable to write index file");
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 90675726484..d8e2fc42e15 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -5,6 +5,8 @@ test_description="Test whether cache-tree is properly updated
 Tests whether various commands properly update and/or rewrite the
 cache-tree extension.
 "
+
+TEST_PASSES_SANITIZE_LEAK=true
  . ./test-lib.sh
 
 cmp_cache_tree () {
-- 
2.37.0.874.g7d3439f13c4

[PATCH 4/9] test-tool urlmatch-normalization: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool urlmatch-normalization", as a result
we can mark the corresponding test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-urlmatch-normalization.c | 11 ++++++++---
 t/t0110-urlmatch-normalization.sh      |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/t/helper/test-urlmatch-normalization.c b/t/helper/test-urlmatch-normalization.c
index 8f4d67e6469..86edd454f5c 100644
--- a/t/helper/test-urlmatch-normalization.c
+++ b/t/helper/test-urlmatch-normalization.c
@@ -5,8 +5,9 @@
 int cmd__urlmatch_normalization(int argc, const char **argv)
 {
 	const char usage[] = "test-tool urlmatch-normalization [-p | -l] <url1> | <url1> <url2>";
-	char *url1, *url2;
+	char *url1 = NULL, *url2 = NULL;
 	int opt_p = 0, opt_l = 0;
+	int ret = 0;
 
 	/*
 	 * For one url, succeed if url_normalize succeeds on it, fail otherwise.
@@ -39,7 +40,7 @@ int cmd__urlmatch_normalization(int argc, const char **argv)
 			printf("%s\n", url1);
 		if (opt_l)
 			printf("%u\n", (unsigned)info.url_len);
-		return 0;
+		goto cleanup;
 	}
 
 	if (opt_p || opt_l)
@@ -47,5 +48,9 @@ int cmd__urlmatch_normalization(int argc, const char **argv)
 
 	url1 = url_normalize(argv[1], NULL);
 	url2 = url_normalize(argv[2], NULL);
-	return (url1 && url2 && !strcmp(url1, url2)) ? 0 : 1;
+	ret = (url1 && url2 && !strcmp(url1, url2)) ? 0 : 1;
+cleanup:
+	free(url1);
+	free(url2);
+	return ret;
 }
diff --git a/t/t0110-urlmatch-normalization.sh b/t/t0110-urlmatch-normalization.sh
index 4dc9fecf724..12d817fbd34 100755
--- a/t/t0110-urlmatch-normalization.sh
+++ b/t/t0110-urlmatch-normalization.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='urlmatch URL normalization'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # The base name of the test url files
-- 
2.37.0.874.g7d3439f13c4

[PATCH 5/9] test-tool regex: call regfree(), fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks in "test-tool regex" which have been there since
c91841594c2 (test-regex: Add a test to check for a bug in the regex
routines, 2012-09-01), as a result we can mark a test as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-regex.c           | 40 ++++++++++++++++++++-------------
 t/t7812-grep-icase-non-ascii.sh |  1 +
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/t/helper/test-regex.c b/t/helper/test-regex.c
index d6f28ca8d14..a37d1f7a546 100644
--- a/t/helper/test-regex.c
+++ b/t/helper/test-regex.c
@@ -24,27 +24,35 @@ static int test_regex_bug(void)
 	char *str = "={}\nfred";
 	regex_t r;
 	regmatch_t m[1];
+	int err = 0;
 
 	if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
 		die("failed regcomp() for pattern '%s'", pat);
-	if (regexec(&r, str, 1, m, 0))
-		die("no match of pattern '%s' to string '%s'", pat, str);
+	if (regexec(&r, str, 1, m, 0)) {
+		err = error("no match of pattern '%s' to string '%s'", pat, str);
+		goto cleanup;
+	}
 
 	/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957  */
-	if (m[0].rm_so == 3) /* matches '\n' when it should not */
-		die("regex bug confirmed: re-build git with NO_REGEX=1");
+	if (m[0].rm_so == 3) { /* matches '\n' when it should not */
+		err = error("regex bug confirmed: re-build git with NO_REGEX=1");
+		goto cleanup;
+	}
 
-	return 0;
+cleanup:
+	regfree(&r);
+	return err < 0 ? 1 : 0;
 }
 
 int cmd__regex(int argc, const char **argv)
 {
 	const char *pat;
 	const char *str;
-	int ret, silent = 0, flags = 0;
+	int silent = 0, flags = 0;
 	regex_t r;
 	regmatch_t m[1];
 	char errbuf[64];
+	int ret = 0;
 
 	argv++;
 	argc--;
@@ -85,27 +93,29 @@ int cmd__regex(int argc, const char **argv)
 	}
 	git_setup_gettext();
 
-	ret = regcomp(&r, pat, flags);
-	if (ret) {
+	if (regcomp(&r, pat, flags)) {
 		if (silent)
-			return ret;
+			return 1;
 
 		regerror(ret, &r, errbuf, sizeof(errbuf));
 		die("failed regcomp() for pattern '%s' (%s)", pat, errbuf);
 	}
 	if (!str)
-		return 0;
+		goto cleanup;
 
-	ret = regexec(&r, str, 1, m, 0);
-	if (ret) {
+	if (regexec(&r, str, 1, m, 0)) {
+		ret = 1;
 		if (silent || ret == REG_NOMATCH)
-			return ret;
+			goto cleanup;
 
 		regerror(ret, &r, errbuf, sizeof(errbuf));
-		die("failed regexec() for subject '%s' (%s)", str, errbuf);
+		error("failed regexec() for subject '%s' (%s)", str, errbuf);
+		goto cleanup;
 	}
 
-	return 0;
+cleanup:
+	regfree(&r);
+	return ret;
 usage:
 	usage("\ttest-tool regex --bug\n"
 	      "\ttest-tool regex [--silent] <pattern>\n"
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index ac7be547145..31c66b63c2c 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -2,6 +2,7 @@
 
 test_description='grep icase on non-English locales'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-gettext.sh
 
 doalarm () {
-- 
2.37.0.874.g7d3439f13c4

Re: [PATCH 5/9] test-tool regex: call regfree(), fix memory leaks

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> diff --git a/t/helper/test-regex.c b/t/helper/test-regex.c
> index d6f28ca8d14..a37d1f7a546 100644
> --- a/t/helper/test-regex.c
> +++ b/t/helper/test-regex.c
> @@ -24,27 +24,35 @@ static int test_regex_bug(void)
>  	char *str = "={}\nfred";
>  	regex_t r;
>  	regmatch_t m[1];
> +	int err = 0;
>  
>  	if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
>  		die("failed regcomp() for pattern '%s'", pat);
> -	if (regexec(&r, str, 1, m, 0))
> -		die("no match of pattern '%s' to string '%s'", pat, str);
> +	if (regexec(&r, str, 1, m, 0)) {
> +		err = error("no match of pattern '%s' to string '%s'", pat, str);
> +		goto cleanup;
> +	}

Hmph.  Does it count as leaking to die() while r is still on the
stack?  I do not mind cleaning everything up thoroughly, but I am
curious if this is really necessary.  Apparently it is, as can be
seen in the patch to t/t7812-grep-icase-non-ascii.sh that allows
us to say "PASSES" with this patch.

> @@ -85,27 +93,29 @@ int cmd__regex(int argc, const char **argv)
>  	}
>  	git_setup_gettext();
>  
> -	ret = regcomp(&r, pat, flags);
> -	if (ret) {
> +	if (regcomp(&r, pat, flags)) {
>  		if (silent)
> -			return ret;
> +			return 1;
>  
>  		regerror(ret, &r, errbuf, sizeof(errbuf));
>  		die("failed regcomp() for pattern '%s' (%s)", pat, errbuf);
>  	}
>  	if (!str)
> -		return 0;
> +		goto cleanup;

Ahh, OK, this one does not bother with regfree() at all.  The
changes to the other one may be irrelevant but the changes to this
function would be necessary to mark the test with "PASSES".

> -	ret = regexec(&r, str, 1, m, 0);
> -	if (ret) {
> +	if (regexec(&r, str, 1, m, 0)) {
> +		ret = 1;
>  		if (silent || ret == REG_NOMATCH)
> -			return ret;
> +			goto cleanup;
>  
>  		regerror(ret, &r, errbuf, sizeof(errbuf));
> -		die("failed regexec() for subject '%s' (%s)", str, errbuf);
> +		error("failed regexec() for subject '%s' (%s)", str, errbuf);
> +		goto cleanup;
>  	}
>  
> -	return 0;
> +cleanup:
> +	regfree(&r);
> +	return ret;
>  usage:
>  	usage("\ttest-tool regex --bug\n"
>  	      "\ttest-tool regex [--silent] <pattern>\n"
> diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
> index ac7be547145..31c66b63c2c 100755
> --- a/t/t7812-grep-icase-non-ascii.sh
> +++ b/t/t7812-grep-icase-non-ascii.sh
> @@ -2,6 +2,7 @@
>  
>  test_description='grep icase on non-English locales'
>  
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./lib-gettext.sh
>  
>  doalarm () {

[PATCH 6/9] test-tool json-writer: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks introduced with these tests in
75459410edd (json_writer: new routines to create JSON data,
2018-07-13), as a result we can mark a test as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-json-writer.c | 16 ++++++++++++----
 t/t0019-json-writer.sh      |  2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/t/helper/test-json-writer.c b/t/helper/test-json-writer.c
index 37c452535f8..8c3edacc000 100644
--- a/t/helper/test-json-writer.c
+++ b/t/helper/test-json-writer.c
@@ -181,12 +181,18 @@ static struct json_writer nest1 = JSON_WRITER_INIT;
 
 static void make_nest1(int pretty)
 {
+	make_obj1(0);
+	make_arr1(0);
+
 	jw_object_begin(&nest1, pretty);
 	{
 		jw_object_sub_jw(&nest1, "obj1", &obj1);
 		jw_object_sub_jw(&nest1, "arr1", &arr1);
 	}
 	jw_end(&nest1);
+
+	jw_release(&obj1);
+	jw_release(&arr1);
 }
 
 static char *expect_inline1 =
@@ -313,6 +319,9 @@ static void make_mixed1(int pretty)
 		jw_object_sub_jw(&mixed1, "arr1", &arr1);
 	}
 	jw_end(&mixed1);
+
+	jw_release(&obj1);
+	jw_release(&arr1);
 }
 
 static void cmp(const char *test, const struct json_writer *jw, const char *exp)
@@ -325,8 +334,8 @@ static void cmp(const char *test, const struct json_writer *jw, const char *exp)
 	exit(1);
 }
 
-#define t(v) do { make_##v(0); cmp(#v, &v, expect_##v); } while (0)
-#define p(v) do { make_##v(1); cmp(#v, &v, pretty_##v); } while (0)
+#define t(v) do { make_##v(0); cmp(#v, &v, expect_##v); jw_release(&v); } while (0)
+#define p(v) do { make_##v(1); cmp(#v, &v, pretty_##v); jw_release(&v); } while (0)
 
 /*
  * Run some basic regression tests with some known patterns.
@@ -381,7 +390,6 @@ static int unit_tests(void)
 
 	/* mixed forms */
 	t(mixed1);
-	jw_init(&mixed1);
 	p(mixed1);
 
 	return 0;
@@ -544,7 +552,7 @@ static int scripted(void)
 
 	printf("%s\n", jw.json.buf);
 
-	strbuf_release(&jw.json);
+	jw_release(&jw);
 	return 0;
 }
 
diff --git a/t/t0019-json-writer.sh b/t/t0019-json-writer.sh
index 3b0c336b38e..19a730c29ed 100755
--- a/t/t0019-json-writer.sh
+++ b/t/t0019-json-writer.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test json-writer JSON generation'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'unit test of json-writer routines' '
-- 
2.37.0.874.g7d3439f13c4

[PATCH 7/9] test-tool bloom: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c:
introduce core Bloom filter constructs, 2020-03-30), as a result we
can mark almost the entirety of t0095-bloom.sh as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true", there's still an
unrelated memory leak in "git commit" in one of the tests, let's skip
that one under SANITIZE_LEAK for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-bloom.c | 2 ++
 t/t0095-bloom.sh      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-bloom.c b/t/helper/test-bloom.c
index ad3ef1cd77a..6c900ca6684 100644
--- a/t/helper/test-bloom.c
+++ b/t/helper/test-bloom.c
@@ -16,6 +16,7 @@ static void add_string_to_filter(const char *data, struct bloom_filter *filter)
 		}
 		printf("\n");
 		add_key_to_filter(&key, filter, &settings);
+		clear_bloom_key(&key);
 }
 
 static void print_bloom_filter(struct bloom_filter *filter) {
@@ -80,6 +81,7 @@ int cmd__bloom(int argc, const char **argv)
 		}
 
 		print_bloom_filter(&filter);
+		free(filter.data);
 	}
 
 	if (!strcmp(argv[1], "get_filter_for_commit")) {
diff --git a/t/t0095-bloom.sh b/t/t0095-bloom.sh
index 5945973552a..daeb4a5e3e7 100755
--- a/t/t0095-bloom.sh
+++ b/t/t0095-bloom.sh
@@ -67,7 +67,7 @@ test_expect_success 'compute bloom key for test string 2' '
 	test_cmp expect actual
 '
 
-test_expect_success 'get bloom filters for commit with no changes' '
+test_expect_success !SANITIZE_LEAK 'get bloom filters for commit with no changes' '
 	git init &&
 	git commit --allow-empty -m "c0" &&
 	cat >expect <<-\EOF &&
-- 
2.37.0.874.g7d3439f13c4

Re: [PATCH 7/9] test-tool bloom: fix a memory leak

[Eric Sunshine]

On Thu, Jun 30, 2022 at 7:51 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> test-tool bloom: fix a memory leak
>
> Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c:

pure nit: the subject talks about a single leak but the message body
talks about multiple leaks.

> introduce core Bloom filter constructs, 2020-03-30), as a result we
> can mark almost the entirety of t0095-bloom.sh as passing with
> SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true", there's still an
> unrelated memory leak in "git commit" in one of the tests, let's skip
> that one under SANITIZE_LEAK for now.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

Re: [PATCH 7/9] test-tool bloom: fix a memory leak

[Ævar Arnfjörð Bjarmason]

On Fri, Jul 01 2022, Eric Sunshine wrote:

> On Thu, Jun 30, 2022 at 7:51 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> test-tool bloom: fix a memory leak
>>
>> Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c:
>
> pure nit: the subject talks about a single leak but the message body
> talks about multiple leaks.

Will fix, I was copy/pasting the commit messages forward & adjusting,
but clearly not enough...

[PATCH 8/9] test-tool ref-store: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak introduced in fa099d23227 (worktree.c: kill
parse_ref() in favor of refs_resolve_ref_unsafe(), 2017-04-24), as a
result we can mark another test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-ref-store.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 9646d85fc84..4d18bfb1ca5 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -96,6 +96,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 			die("no such worktree: %s", gitdir);
 
 		*refs = get_worktree_ref_store(*p);
+		free_worktrees(worktrees);
 	} else
 		die("unknown backend %s", argv[0]);
 
-- 
2.37.0.874.g7d3439f13c4

[PATCH 9/9] test-tool delta: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak introduced in a310d434946 ([PATCH] Deltification
library work by Nicolas Pitre., 2005-05-19), as a result we can mark
another test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-delta.c                 | 21 ++++++++++++++-------
 t/t5303-pack-corruption-resilience.sh |  2 ++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/t/helper/test-delta.c b/t/helper/test-delta.c
index e749a49c88e..b15481ea596 100644
--- a/t/helper/test-delta.c
+++ b/t/helper/test-delta.c
@@ -20,8 +20,9 @@ int cmd__delta(int argc, const char **argv)
 {
 	int fd;
 	struct stat st;
-	void *from_buf, *data_buf, *out_buf;
+	void *from_buf = NULL, *data_buf = NULL, *out_buf = NULL;
 	unsigned long from_size, data_size, out_size;
+	int ret = 1;
 
 	if (argc != 5 || (strcmp(argv[1], "-d") && strcmp(argv[1], "-p"))) {
 		fprintf(stderr, "usage: %s\n", usage_str);
@@ -38,21 +39,21 @@ int cmd__delta(int argc, const char **argv)
 	if (read_in_full(fd, from_buf, from_size) < 0) {
 		perror(argv[2]);
 		close(fd);
-		return 1;
+		goto cleanup;
 	}
 	close(fd);
 
 	fd = open(argv[3], O_RDONLY);
 	if (fd < 0 || fstat(fd, &st)) {
 		perror(argv[3]);
-		return 1;
+		goto cleanup;
 	}
 	data_size = st.st_size;
 	data_buf = xmalloc(data_size);
 	if (read_in_full(fd, data_buf, data_size) < 0) {
 		perror(argv[3]);
 		close(fd);
-		return 1;
+		goto cleanup;
 	}
 	close(fd);
 
@@ -66,14 +67,20 @@ int cmd__delta(int argc, const char **argv)
 				      &out_size);
 	if (!out_buf) {
 		fprintf(stderr, "delta operation failed (returned NULL)\n");
-		return 1;
+		goto cleanup;
 	}
 
 	fd = open (argv[4], O_WRONLY|O_CREAT|O_TRUNC, 0666);
 	if (fd < 0 || write_in_full(fd, out_buf, out_size) < 0) {
 		perror(argv[4]);
-		return 1;
+		goto cleanup;
 	}
 
-	return 0;
+	ret = 0;
+cleanup:
+	free(from_buf);
+	free(data_buf);
+	free(out_buf);
+
+	return ret;
 }
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 41e6dc4dcfc..2926e8dfc41 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -4,6 +4,8 @@
 #
 
 test_description='resilience to pack corruptions with redundant objects'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Note: the test objects are created with knowledge of their pack encoding
-- 
2.37.0.874.g7d3439f13c4

[PATCH v2 0/9] test-tool: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks in "test-tool", see the v1 CL for a general summary:
https://lore.kernel.org/git/cover-0.9-00000000000-20220630T180129Z-avarab@gmail.com/

Changes since v1:

 * Use a less stupid "to_free" pattern in 2/9 (thanks Eric!)
 * Don't go overboard with freeing memory before die() in 5/9, thanks
   Junio.
 * Fix the commit message of 7/9 as suggested by Eric.

Ævar Arnfjörð Bjarmason (9):
  test-tool test-hash: fix a memory leak
  test-tool path-utils: fix a memory leak
  test-tool {dump,scrap}-cache-tree: fix memory leaks
  test-tool urlmatch-normalization: fix a memory leak
  test-tool regex: call regfree(), fix memory leaks
  test-tool json-writer: fix memory leaks
  test-tool bloom: fix memory leaks
  test-tool ref-store: fix a memory leak
  test-tool delta: fix a memory leak

 t/helper/test-bloom.c                  |  2 ++
 t/helper/test-delta.c                  | 21 ++++++++++++++-------
 t/helper/test-dump-cache-tree.c        |  7 ++++++-
 t/helper/test-hash.c                   |  1 +
 t/helper/test-json-writer.c            | 16 ++++++++++++----
 t/helper/test-path-utils.c             | 11 +++++++----
 t/helper/test-ref-store.c              |  1 +
 t/helper/test-regex.c                  |  9 ++++++---
 t/helper/test-scrap-cache-tree.c       |  1 +
 t/helper/test-urlmatch-normalization.c | 11 ++++++++---
 t/t0015-hash.sh                        |  3 ++-
 t/t0019-json-writer.sh                 |  2 ++
 t/t0060-path-utils.sh                  |  1 +
 t/t0090-cache-tree.sh                  |  2 ++
 t/t0095-bloom.sh                       |  2 +-
 t/t0110-urlmatch-normalization.sh      |  2 ++
 t/t5303-pack-corruption-resilience.sh  |  2 ++
 t/t5308-pack-detect-duplicates.sh      |  2 ++
 t/t5309-pack-delta-cycles.sh           |  2 ++
 t/t5321-pack-large-objects.sh          |  2 ++
 t/t7812-grep-icase-non-ascii.sh        |  1 +
 21 files changed, 77 insertions(+), 24 deletions(-)

Range-diff against v1:
 -:  ----------- >  1:  4406fedba80 test-tool test-hash: fix a memory leak
 1:  06b2dcf4f12 !  2:  050766e6fa2 test-tool path-utils: fix a memory leak
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/helper/test-path-utils.c ##
    -@@ t/helper/test-path-utils.c: static int protect_ntfs_hfs_benchmark(int argc, const char **argv)
    - int cmd__path_utils(int argc, const char **argv)
    - {
    +@@ t/helper/test-path-utils.c: int cmd__path_utils(int argc, const char **argv)
      	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
    --		char *buf = xmallocz(strlen(argv[2]));
    -+		char *to_free = NULL;
    -+		char *buf = to_free = xmallocz(strlen(argv[2]));
    + 		char *buf = xmallocz(strlen(argv[2]));
      		int rv = normalize_path_copy(buf, argv[2]);
    - 		if (rv)
    - 			buf = "++failed++";
    - 		puts(buf);
    -+		free(to_free);
    +-		if (rv)
    +-			buf = "++failed++";
    +-		puts(buf);
    ++		puts(rv ? "++failed++" : buf);
    ++		free(buf);
      		return 0;
      	}
      
 2:  7a0064860ad =  3:  5f9f34629c4 test-tool {dump,scrap}-cache-tree: fix memory leaks
 3:  a450aff8904 =  4:  3f9f7bbdeb2 test-tool urlmatch-normalization: fix a memory leak
 4:  fe2a8d898f6 !  5:  940cd5378ad test-tool regex: call regfree(), fix memory leaks
    @@ Commit message
         routines, 2012-09-01), as a result we can mark a test as passing with
         SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true".
     
    +    We could regfree() on the die() paths here, which would make some
    +    invocations of valgrind(1) happy, but let's just target SANITIZE=leak
    +    for now. Variables that are still reachable when we die() are not
    +    reported as leaks.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/helper/test-regex.c ##
     @@ t/helper/test-regex.c: static int test_regex_bug(void)
    - 	char *str = "={}\nfred";
    - 	regex_t r;
    - 	regmatch_t m[1];
    -+	int err = 0;
    - 
    - 	if (regcomp(&r, pat, REG_EXTENDED | REG_NEWLINE))
    - 		die("failed regcomp() for pattern '%s'", pat);
    --	if (regexec(&r, str, 1, m, 0))
    --		die("no match of pattern '%s' to string '%s'", pat, str);
    -+	if (regexec(&r, str, 1, m, 0)) {
    -+		err = error("no match of pattern '%s' to string '%s'", pat, str);
    -+		goto cleanup;
    -+	}
    + 	if (m[0].rm_so == 3) /* matches '\n' when it should not */
    + 		die("regex bug confirmed: re-build git with NO_REGEX=1");
      
    - 	/* http://sourceware.org/bugzilla/show_bug.cgi?id=3957  */
    --	if (m[0].rm_so == 3) /* matches '\n' when it should not */
    --		die("regex bug confirmed: re-build git with NO_REGEX=1");
    -+	if (m[0].rm_so == 3) { /* matches '\n' when it should not */
    -+		err = error("regex bug confirmed: re-build git with NO_REGEX=1");
    -+		goto cleanup;
    -+	}
    - 
    --	return 0;
    -+cleanup:
     +	regfree(&r);
    -+	return err < 0 ? 1 : 0;
    + 	return 0;
      }
      
    - int cmd__regex(int argc, const char **argv)
    - {
    - 	const char *pat;
    - 	const char *str;
    --	int ret, silent = 0, flags = 0;
    -+	int silent = 0, flags = 0;
    - 	regex_t r;
    - 	regmatch_t m[1];
    - 	char errbuf[64];
    -+	int ret = 0;
    - 
    - 	argv++;
    - 	argc--;
     @@ t/helper/test-regex.c: int cmd__regex(int argc, const char **argv)
    - 	}
    - 	git_setup_gettext();
    - 
    --	ret = regcomp(&r, pat, flags);
    --	if (ret) {
    -+	if (regcomp(&r, pat, flags)) {
    - 		if (silent)
    --			return ret;
    -+			return 1;
    - 
    - 		regerror(ret, &r, errbuf, sizeof(errbuf));
      		die("failed regcomp() for pattern '%s' (%s)", pat, errbuf);
      	}
      	if (!str)
     -		return 0;
     +		goto cleanup;
      
    --	ret = regexec(&r, str, 1, m, 0);
    --	if (ret) {
    -+	if (regexec(&r, str, 1, m, 0)) {
    -+		ret = 1;
    + 	ret = regexec(&r, str, 1, m, 0);
    + 	if (ret) {
      		if (silent || ret == REG_NOMATCH)
     -			return ret;
     +			goto cleanup;
      
      		regerror(ret, &r, errbuf, sizeof(errbuf));
    --		die("failed regexec() for subject '%s' (%s)", str, errbuf);
    -+		error("failed regexec() for subject '%s' (%s)", str, errbuf);
    -+		goto cleanup;
    + 		die("failed regexec() for subject '%s' (%s)", str, errbuf);
      	}
      
     -	return 0;
 5:  97448b9056e =  6:  24022fc2b3f test-tool json-writer: fix memory leaks
 6:  bdb467d1414 !  7:  0957c99817b test-tool bloom: fix a memory leak
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Commit message ##
    -    test-tool bloom: fix a memory leak
    +    test-tool bloom: fix memory leaks
     
         Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c:
         introduce core Bloom filter constructs, 2020-03-30), as a result we
 7:  6becefc754e =  8:  937b4cdf07e test-tool ref-store: fix a memory leak
 8:  5341b413bda =  9:  0968f549957 test-tool delta: fix a memory leak
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 1/9] test-tool test-hash: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool test-hash" which has been there since
b57cbbf8a86 (test-sha1: test hashing large buffer, 2006-06-24), as a
result we can mark more tests as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-hash.c              | 1 +
 t/t0015-hash.sh                   | 3 ++-
 t/t5308-pack-detect-duplicates.sh | 2 ++
 t/t5309-pack-delta-cycles.sh      | 2 ++
 t/t5321-pack-large-objects.sh     | 2 ++
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-hash.c b/t/helper/test-hash.c
index 261c545b9d1..5860dab0ffa 100644
--- a/t/helper/test-hash.c
+++ b/t/helper/test-hash.c
@@ -54,5 +54,6 @@ int cmd_hash_impl(int ac, const char **av, int algo)
 		fwrite(hash, 1, algop->rawsz, stdout);
 	else
 		puts(hash_to_hex_algop(hash, algop));
+	free(buffer);
 	return 0;
 }
diff --git a/t/t0015-hash.sh b/t/t0015-hash.sh
index 086822fc45b..0a087a1983d 100755
--- a/t/t0015-hash.sh
+++ b/t/t0015-hash.sh
@@ -1,8 +1,9 @@
 #!/bin/sh
 
 test_description='test basic hash implementation'
-. ./test-lib.sh
 
+TEST_PASSES_SANITIZE_LEAK=true
+. ./test-lib.sh
 
 test_expect_success 'test basic SHA-1 hash values' '
 	test-tool sha1 </dev/null >actual &&
diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
index 693b2411c89..655cafa0541 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='handling of duplicate objects in incoming packfiles'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
index 55b787630fc..4e910c5b9d2 100755
--- a/t/t5309-pack-delta-cycles.sh
+++ b/t/t5309-pack-delta-cycles.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test index-pack handling of delta cycles in packfiles'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
diff --git a/t/t5321-pack-large-objects.sh b/t/t5321-pack-large-objects.sh
index 8a56d98a0e8..70770fe274d 100755
--- a/t/t5321-pack-large-objects.sh
+++ b/t/t5321-pack-large-objects.sh
@@ -6,6 +6,8 @@
 test_description='git pack-object with "large" deltas
 
 '
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-pack.sh
 
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 2/9] test-tool path-utils: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool path-utils", as a result we can mark
the corresponding test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-path-utils.c | 11 +++++++----
 t/t0060-path-utils.sh      |  1 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index 229ed416b0e..d20e1b7a18d 100644
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -296,9 +296,8 @@ int cmd__path_utils(int argc, const char **argv)
 	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
 		char *buf = xmallocz(strlen(argv[2]));
 		int rv = normalize_path_copy(buf, argv[2]);
-		if (rv)
-			buf = "++failed++";
-		puts(buf);
+		puts(rv ? "++failed++" : buf);
+		free(buf);
 		return 0;
 	}
 
@@ -356,7 +355,10 @@ int cmd__path_utils(int argc, const char **argv)
 		int nongit_ok;
 		setup_git_directory_gently(&nongit_ok);
 		while (argc > 3) {
-			puts(prefix_path(prefix, prefix_len, argv[3]));
+			char *pfx = prefix_path(prefix, prefix_len, argv[3]);
+
+			puts(pfx);
+			free(pfx);
 			argc--;
 			argv++;
 		}
@@ -366,6 +368,7 @@ int cmd__path_utils(int argc, const char **argv)
 	if (argc == 4 && !strcmp(argv[1], "strip_path_suffix")) {
 		char *prefix = strip_path_suffix(argv[2], argv[3]);
 		printf("%s\n", prefix ? prefix : "(null)");
+		free(prefix);
 		return 0;
 	}
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index aa35350b6f3..1f2007e62b7 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -5,6 +5,7 @@
 
 test_description='Test various path utilities'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 norm_path() {
-- 
2.37.0.900.g4d0de1cceb2

Re: [PATCH v2 2/9] test-tool path-utils: fix a memory leak

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Fix a memory leak in "test-tool path-utils", as a result we can mark
> the corresponding test as passing with SANITIZE=leak using
> "TEST_PASSES_SANITIZE_LEAK=true".
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  t/helper/test-path-utils.c | 11 +++++++----
>  t/t0060-path-utils.sh      |  1 +
>  2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
> index 229ed416b0e..d20e1b7a18d 100644
> --- a/t/helper/test-path-utils.c
> +++ b/t/helper/test-path-utils.c
> @@ -296,9 +296,8 @@ int cmd__path_utils(int argc, const char **argv)
>  	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
>  		char *buf = xmallocz(strlen(argv[2]));
>  		int rv = normalize_path_copy(buf, argv[2]);
> -		if (rv)
> -			buf = "++failed++";
> -		puts(buf);
> +		puts(rv ? "++failed++" : buf);
> +		free(buf);

This version, without the need for to_free, is certainly very easy
to understand.  Nicely done.

[PATCH v2 3/9] test-tool {dump,scrap}-cache-tree: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks in two test-tools used by t0090-cache-tree.sh. As a
result we can mark the test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-dump-cache-tree.c  | 7 ++++++-
 t/helper/test-scrap-cache-tree.c | 1 +
 t/t0090-cache-tree.sh            | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-dump-cache-tree.c b/t/helper/test-dump-cache-tree.c
index 6a3f88f5f5d..0d6d7f1ecbf 100644
--- a/t/helper/test-dump-cache-tree.c
+++ b/t/helper/test-dump-cache-tree.c
@@ -59,11 +59,16 @@ int cmd__dump_cache_tree(int ac, const char **av)
 {
 	struct index_state istate;
 	struct cache_tree *another = cache_tree();
+	int ret;
+
 	setup_git_directory();
 	if (read_cache() < 0)
 		die("unable to read index file");
 	istate = the_index;
 	istate.cache_tree = another;
 	cache_tree_update(&istate, WRITE_TREE_DRY_RUN);
-	return dump_cache_tree(active_cache_tree, another, "");
+	ret = dump_cache_tree(active_cache_tree, another, "");
+	cache_tree_free(&another);
+
+	return ret;
 }
diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index 393f1604ff9..026c802479d 100644
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -12,6 +12,7 @@ int cmd__scrap_cache_tree(int ac, const char **av)
 	hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
 	if (read_cache() < 0)
 		die("unable to read index file");
+	cache_tree_free(&active_cache_tree);
 	active_cache_tree = NULL;
 	if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
 		die("unable to write index file");
diff --git a/t/t0090-cache-tree.sh b/t/t0090-cache-tree.sh
index 90675726484..d8e2fc42e15 100755
--- a/t/t0090-cache-tree.sh
+++ b/t/t0090-cache-tree.sh
@@ -5,6 +5,8 @@ test_description="Test whether cache-tree is properly updated
 Tests whether various commands properly update and/or rewrite the
 cache-tree extension.
 "
+
+TEST_PASSES_SANITIZE_LEAK=true
  . ./test-lib.sh
 
 cmp_cache_tree () {
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 4/9] test-tool urlmatch-normalization: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak in "test-tool urlmatch-normalization", as a result
we can mark the corresponding test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-urlmatch-normalization.c | 11 ++++++++---
 t/t0110-urlmatch-normalization.sh      |  2 ++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/t/helper/test-urlmatch-normalization.c b/t/helper/test-urlmatch-normalization.c
index 8f4d67e6469..86edd454f5c 100644
--- a/t/helper/test-urlmatch-normalization.c
+++ b/t/helper/test-urlmatch-normalization.c
@@ -5,8 +5,9 @@
 int cmd__urlmatch_normalization(int argc, const char **argv)
 {
 	const char usage[] = "test-tool urlmatch-normalization [-p | -l] <url1> | <url1> <url2>";
-	char *url1, *url2;
+	char *url1 = NULL, *url2 = NULL;
 	int opt_p = 0, opt_l = 0;
+	int ret = 0;
 
 	/*
 	 * For one url, succeed if url_normalize succeeds on it, fail otherwise.
@@ -39,7 +40,7 @@ int cmd__urlmatch_normalization(int argc, const char **argv)
 			printf("%s\n", url1);
 		if (opt_l)
 			printf("%u\n", (unsigned)info.url_len);
-		return 0;
+		goto cleanup;
 	}
 
 	if (opt_p || opt_l)
@@ -47,5 +48,9 @@ int cmd__urlmatch_normalization(int argc, const char **argv)
 
 	url1 = url_normalize(argv[1], NULL);
 	url2 = url_normalize(argv[2], NULL);
-	return (url1 && url2 && !strcmp(url1, url2)) ? 0 : 1;
+	ret = (url1 && url2 && !strcmp(url1, url2)) ? 0 : 1;
+cleanup:
+	free(url1);
+	free(url2);
+	return ret;
 }
diff --git a/t/t0110-urlmatch-normalization.sh b/t/t0110-urlmatch-normalization.sh
index 4dc9fecf724..12d817fbd34 100755
--- a/t/t0110-urlmatch-normalization.sh
+++ b/t/t0110-urlmatch-normalization.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='urlmatch URL normalization'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # The base name of the test url files
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 5/9] test-tool regex: call regfree(), fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks in "test-tool regex" which have been there since
c91841594c2 (test-regex: Add a test to check for a bug in the regex
routines, 2012-09-01), as a result we can mark a test as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true".

We could regfree() on the die() paths here, which would make some
invocations of valgrind(1) happy, but let's just target SANITIZE=leak
for now. Variables that are still reachable when we die() are not
reported as leaks.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-regex.c           | 9 ++++++---
 t/t7812-grep-icase-non-ascii.sh | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/t/helper/test-regex.c b/t/helper/test-regex.c
index d6f28ca8d14..bd871a735b4 100644
--- a/t/helper/test-regex.c
+++ b/t/helper/test-regex.c
@@ -34,6 +34,7 @@ static int test_regex_bug(void)
 	if (m[0].rm_so == 3) /* matches '\n' when it should not */
 		die("regex bug confirmed: re-build git with NO_REGEX=1");
 
+	regfree(&r);
 	return 0;
 }
 
@@ -94,18 +95,20 @@ int cmd__regex(int argc, const char **argv)
 		die("failed regcomp() for pattern '%s' (%s)", pat, errbuf);
 	}
 	if (!str)
-		return 0;
+		goto cleanup;
 
 	ret = regexec(&r, str, 1, m, 0);
 	if (ret) {
 		if (silent || ret == REG_NOMATCH)
-			return ret;
+			goto cleanup;
 
 		regerror(ret, &r, errbuf, sizeof(errbuf));
 		die("failed regexec() for subject '%s' (%s)", str, errbuf);
 	}
 
-	return 0;
+cleanup:
+	regfree(&r);
+	return ret;
 usage:
 	usage("\ttest-tool regex --bug\n"
 	      "\ttest-tool regex [--silent] <pattern>\n"
diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh
index ac7be547145..31c66b63c2c 100755
--- a/t/t7812-grep-icase-non-ascii.sh
+++ b/t/t7812-grep-icase-non-ascii.sh
@@ -2,6 +2,7 @@
 
 test_description='grep icase on non-English locales'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./lib-gettext.sh
 
 doalarm () {
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 6/9] test-tool json-writer: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks introduced with these tests in
75459410edd (json_writer: new routines to create JSON data,
2018-07-13), as a result we can mark a test as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-json-writer.c | 16 ++++++++++++----
 t/t0019-json-writer.sh      |  2 ++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/t/helper/test-json-writer.c b/t/helper/test-json-writer.c
index 37c452535f8..8c3edacc000 100644
--- a/t/helper/test-json-writer.c
+++ b/t/helper/test-json-writer.c
@@ -181,12 +181,18 @@ static struct json_writer nest1 = JSON_WRITER_INIT;
 
 static void make_nest1(int pretty)
 {
+	make_obj1(0);
+	make_arr1(0);
+
 	jw_object_begin(&nest1, pretty);
 	{
 		jw_object_sub_jw(&nest1, "obj1", &obj1);
 		jw_object_sub_jw(&nest1, "arr1", &arr1);
 	}
 	jw_end(&nest1);
+
+	jw_release(&obj1);
+	jw_release(&arr1);
 }
 
 static char *expect_inline1 =
@@ -313,6 +319,9 @@ static void make_mixed1(int pretty)
 		jw_object_sub_jw(&mixed1, "arr1", &arr1);
 	}
 	jw_end(&mixed1);
+
+	jw_release(&obj1);
+	jw_release(&arr1);
 }
 
 static void cmp(const char *test, const struct json_writer *jw, const char *exp)
@@ -325,8 +334,8 @@ static void cmp(const char *test, const struct json_writer *jw, const char *exp)
 	exit(1);
 }
 
-#define t(v) do { make_##v(0); cmp(#v, &v, expect_##v); } while (0)
-#define p(v) do { make_##v(1); cmp(#v, &v, pretty_##v); } while (0)
+#define t(v) do { make_##v(0); cmp(#v, &v, expect_##v); jw_release(&v); } while (0)
+#define p(v) do { make_##v(1); cmp(#v, &v, pretty_##v); jw_release(&v); } while (0)
 
 /*
  * Run some basic regression tests with some known patterns.
@@ -381,7 +390,6 @@ static int unit_tests(void)
 
 	/* mixed forms */
 	t(mixed1);
-	jw_init(&mixed1);
 	p(mixed1);
 
 	return 0;
@@ -544,7 +552,7 @@ static int scripted(void)
 
 	printf("%s\n", jw.json.buf);
 
-	strbuf_release(&jw.json);
+	jw_release(&jw);
 	return 0;
 }
 
diff --git a/t/t0019-json-writer.sh b/t/t0019-json-writer.sh
index 3b0c336b38e..19a730c29ed 100755
--- a/t/t0019-json-writer.sh
+++ b/t/t0019-json-writer.sh
@@ -1,6 +1,8 @@
 #!/bin/sh
 
 test_description='test json-writer JSON generation'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'unit test of json-writer routines' '
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 7/9] test-tool bloom: fix memory leaks

[Ævar Arnfjörð Bjarmason]

Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c:
introduce core Bloom filter constructs, 2020-03-30), as a result we
can mark almost the entirety of t0095-bloom.sh as passing with
SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true", there's still an
unrelated memory leak in "git commit" in one of the tests, let's skip
that one under SANITIZE_LEAK for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-bloom.c | 2 ++
 t/t0095-bloom.sh      | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/t/helper/test-bloom.c b/t/helper/test-bloom.c
index ad3ef1cd77a..6c900ca6684 100644
--- a/t/helper/test-bloom.c
+++ b/t/helper/test-bloom.c
@@ -16,6 +16,7 @@ static void add_string_to_filter(const char *data, struct bloom_filter *filter)
 		}
 		printf("\n");
 		add_key_to_filter(&key, filter, &settings);
+		clear_bloom_key(&key);
 }
 
 static void print_bloom_filter(struct bloom_filter *filter) {
@@ -80,6 +81,7 @@ int cmd__bloom(int argc, const char **argv)
 		}
 
 		print_bloom_filter(&filter);
+		free(filter.data);
 	}
 
 	if (!strcmp(argv[1], "get_filter_for_commit")) {
diff --git a/t/t0095-bloom.sh b/t/t0095-bloom.sh
index 5945973552a..daeb4a5e3e7 100755
--- a/t/t0095-bloom.sh
+++ b/t/t0095-bloom.sh
@@ -67,7 +67,7 @@ test_expect_success 'compute bloom key for test string 2' '
 	test_cmp expect actual
 '
 
-test_expect_success 'get bloom filters for commit with no changes' '
+test_expect_success !SANITIZE_LEAK 'get bloom filters for commit with no changes' '
 	git init &&
 	git commit --allow-empty -m "c0" &&
 	cat >expect <<-\EOF &&
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 8/9] test-tool ref-store: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak introduced in fa099d23227 (worktree.c: kill
parse_ref() in favor of refs_resolve_ref_unsafe(), 2017-04-24), as a
result we can mark another test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-ref-store.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 9646d85fc84..4d18bfb1ca5 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -96,6 +96,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 			die("no such worktree: %s", gitdir);
 
 		*refs = get_worktree_ref_store(*p);
+		free_worktrees(worktrees);
 	} else
 		die("unknown backend %s", argv[0]);
 
-- 
2.37.0.900.g4d0de1cceb2

[PATCH v2 9/9] test-tool delta: fix a memory leak

[Ævar Arnfjörð Bjarmason]

Fix a memory leak introduced in a310d434946 ([PATCH] Deltification
library work by Nicolas Pitre., 2005-05-19), as a result we can mark
another test as passing with SANITIZE=leak using
"TEST_PASSES_SANITIZE_LEAK=true".

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/helper/test-delta.c                 | 21 ++++++++++++++-------
 t/t5303-pack-corruption-resilience.sh |  2 ++
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/t/helper/test-delta.c b/t/helper/test-delta.c
index e749a49c88e..b15481ea596 100644
--- a/t/helper/test-delta.c
+++ b/t/helper/test-delta.c
@@ -20,8 +20,9 @@ int cmd__delta(int argc, const char **argv)
 {
 	int fd;
 	struct stat st;
-	void *from_buf, *data_buf, *out_buf;
+	void *from_buf = NULL, *data_buf = NULL, *out_buf = NULL;
 	unsigned long from_size, data_size, out_size;
+	int ret = 1;
 
 	if (argc != 5 || (strcmp(argv[1], "-d") && strcmp(argv[1], "-p"))) {
 		fprintf(stderr, "usage: %s\n", usage_str);
@@ -38,21 +39,21 @@ int cmd__delta(int argc, const char **argv)
 	if (read_in_full(fd, from_buf, from_size) < 0) {
 		perror(argv[2]);
 		close(fd);
-		return 1;
+		goto cleanup;
 	}
 	close(fd);
 
 	fd = open(argv[3], O_RDONLY);
 	if (fd < 0 || fstat(fd, &st)) {
 		perror(argv[3]);
-		return 1;
+		goto cleanup;
 	}
 	data_size = st.st_size;
 	data_buf = xmalloc(data_size);
 	if (read_in_full(fd, data_buf, data_size) < 0) {
 		perror(argv[3]);
 		close(fd);
-		return 1;
+		goto cleanup;
 	}
 	close(fd);
 
@@ -66,14 +67,20 @@ int cmd__delta(int argc, const char **argv)
 				      &out_size);
 	if (!out_buf) {
 		fprintf(stderr, "delta operation failed (returned NULL)\n");
-		return 1;
+		goto cleanup;
 	}
 
 	fd = open (argv[4], O_WRONLY|O_CREAT|O_TRUNC, 0666);
 	if (fd < 0 || write_in_full(fd, out_buf, out_size) < 0) {
 		perror(argv[4]);
-		return 1;
+		goto cleanup;
 	}
 
-	return 0;
+	ret = 0;
+cleanup:
+	free(from_buf);
+	free(data_buf);
+	free(out_buf);
+
+	return ret;
 }
diff --git a/t/t5303-pack-corruption-resilience.sh b/t/t5303-pack-corruption-resilience.sh
index 41e6dc4dcfc..2926e8dfc41 100755
--- a/t/t5303-pack-corruption-resilience.sh
+++ b/t/t5303-pack-corruption-resilience.sh
@@ -4,6 +4,8 @@
 #
 
 test_description='resilience to pack corruptions with redundant objects'
+
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 # Note: the test objects are created with knowledge of their pack encoding
-- 
2.37.0.900.g4d0de1cceb2