Thread (1 message) 1 message, 1 author, 2022-07-01

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

From: Junio C Hamano <hidden>
Date: 2022-07-01 02:17:54

Ævar Arnfjörð Bjarmason  [off-list ref] writes:
quoted hunk
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.
quoted hunk
@@ -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".
quoted hunk
-	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 () {
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help