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

[Junio C Hamano <gitster@pobox.com>]

Æ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 () {