Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Sorry for being late.  Just wanted to try out this new feature and
ended up reading this old thread.

Am 15.09.2012 01:18, schrieb Junio C Hamano:
>   t/perf/perf-lib.sh |  1 +
>   t/test-lib.sh      | 26 ++++++++++++++++++++------
>   2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git i/t/perf/perf-lib.sh w/t/perf/perf-lib.sh
> index a1361e5..1d0bb9d 100644
> --- i/t/perf/perf-lib.sh
> +++ w/t/perf/perf-lib.sh
> @@ -42,6 +42,7 @@ else
>  fi
>   
>  TEST_NO_CREATE_REPO=t
> +TEST_NO_MALLOC_=t

Why the trailing underscore?  Perhaps add "CHECK" before the equal sign?

>   . ../test-lib.sh
>   
> diff --git i/t/test-lib.sh w/t/test-lib.sh
> index b0c0c84..aad4606 100644
> --- i/t/test-lib.sh
> +++ w/t/test-lib.sh
> @@ -95,12 +95,24 @@ export EDITOR
>  
>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind
> -expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {
> -	MALLOC_CHECK_=3
> -	export MALLOC_CHECK_
> -	MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
> -	export MALLOC_PERTURB_
> -}
> +if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
> +   test -n "TEST_NO_MALLOC_"

Without a $, you'll get nothing. ;-)

('test -n "string"' is always true, unlike 'test -n "$variable"'.)

> +then
> +	setup_malloc_check () {
> +		: nothing
> +	}
> +	teardown_malloc_check () {
> +		: nothing
> +	}
> +else
> +	setup_malloc_check () {
> +		MALLOC_CHECK_=3	MALLOC_PERTURB_=165
> +		export MALLOC_CHECK_ MALLOC_PERTURB_
> +	}
> +	teardown_malloc_check () {
> +		unset MALLOC_CHECK_ MALLOC_PERTURB_
> +	}

Would it make sense to restore the previous values?  Hrm, can't think of a use case.

> +fi
>   
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -311,7 +323,9 @@ test_run_ () {
>  
>  	if test -z "$immediate" || test $eval_ret = 0 || test -n "$expecting_failure"
>  	then
> +		setup_malloc_check
>  		test_eval_ "$test_cleanup"
> +		teardown_malloc_check
>  	fi
>  	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"
>  	then

-- >8 --
Subject: [PATCH] MALLOC_CHECK: enable it, unless disabled explicitly

The malloc checks in tests are currently disabled.  Actually evaluate
the variable for turning them off and enable them if it's unset.

Also use this opportunity to give it the more descriptive and
consistent name TEST_NO_MALLOC_CHECK.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 t/perf/perf-lib.sh | 2 +-
 t/test-lib.sh      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 1d0bb9d..a816fbc 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -42,7 +42,7 @@ else
 fi
 
 TEST_NO_CREATE_REPO=t
-TEST_NO_MALLOC_=t
+TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bff3d75..617d831 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -105,7 +105,7 @@ export EDITOR
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-   test -n "TEST_NO_MALLOC_"
+   test -n "$TEST_NO_MALLOC_CHECK"
 then
 	setup_malloc_check () {
 		: nothing
-- 
1.7.12