[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] suggestion enhancement valgrind_tests.m4
From: |
Bernd Becker |
Subject: |
Re: [PATCH] suggestion enhancement valgrind_tests.m4 |
Date: |
Sat, 20 Nov 2010 17:07:14 +0100 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; de; rv:1.9.1.15) Gecko/20101026 SUSE/3.0.10 Thunderbird/3.0.10 ThunderBrowse/3.3.2 |
Hi Simon,
please find my answers inline.
>> ++AC_DEFUN([gl_VALGRIND_CONSISTENCY],
>> +[
>> + if test "$opt_valgrind_tests" = "no" && test
>> "$opt_valgrind_memleaks_are_errors" != "no"; then
>> + AC_MSG_WARN(
>> + [
>> + *******************************************************************
>> + *** ***
>> + *** Memory leak checking with Valgrind is disabled. ***
>> + *** ***
>> + *** Ignoring: --enable-valgrind-tests ***
>> + *** Ignoring: settings in \$VALGRIND_MEMLEAK_OPTS ***
>> + *** ***
>> + *******************************************************************
>> + ])
>> + fi
>> +])
>>
> This is noisy, especially since the majority of users won't need to
> build using valgrind. You could put this into your project's
> configure.ac if you want it.
>
>
yes you are right. I removed it from the new patch (below).
>> +AC_DEFUN([gl_VALGRIND_SUMMARY],
>> +[
>> + valgrind_options=""
>> + if test "$opt_valgrind_tests" = "yes"; then
>> + if test "$opt_valgrind_memleaks_are_errors" = "yes" ||
>> + test "$opt_valgrind_memleaks_are_errors" = "no";
>> + then
>> + valgrind_options="Memleaks are errors:
>> ${opt_valgrind_memleaks_are_errors}"
>> + else
>> + valgrind_options="Valgrind options:
>> ${opt_valgrind_memleaks_are_errors}"
>> + fi
>> + fi
>> +])
>>
> What does this do?
>
>
This creates the possibility to print the valgrind options into a
configure summary like e.g. in the following
example: I did that by overwriting the opt_valgrind_memleaks_are_errors
in an earlier macro.
AC_MSG_NOTICE([summary of build options:
Host type: ${host}
Install prefix: ${prefix}
Compiler: ${CC}
Library types: Shared=${enable_shared}, Static=${enable_static}
Valgrind: ${opt_valgrind_tests}
${valgrind_options}
])
Note: I renamed valgrind_options to valgrind_summary in the attached patch.
>> +
>> # gl_VALGRIND_TESTS()
>> # -------------------
>> # Check if valgrind is available, and set VALGRIND to it if available.
>> @@ -14,21 +78,27 @@ AC_DEFUN([gl_VALGRIND_TESTS],
>> AC_ARG_ENABLE(valgrind-tests,
>> AS_HELP_STRING([--enable-valgrind-tests],
>> [run self tests under valgrind]),
>> - [opt_valgrind_tests=$enableval], [opt_valgrind_tests=yes])
>> + [opt_valgrind_tests=$enableval], [opt_valgrind_tests=no])
>>
> Why change the default?
>
>
The idea was to explicitly enable valgrind tests. Usually searching for
memory leaks takes a lot of time and when doing regression during e.g. a
"test-first" development approach I usually don't want to spend the
extra time for memory leak testing when checking the logic of the
program. I prefer to make leak checks at the end of feature
implementation. Thus spending the extra time only once.
>From my perspective it is not a big deal to keep the "yes" default.
Maybe it's even better to have it enabled by default.
Changed in the attached patch.
>> # Run self-tests under valgrind?
>> if test "$opt_valgrind_tests" = "yes" && test "$cross_compiling" =
>> no; then
>> AC_CHECK_PROGS(VALGRIND, valgrind)
>> fi
>>
>> - if test -n "$VALGRIND" && $VALGRIND -q true > /dev/null 2>&1; then
>> + if test -n "$VALGRIND" && $VALGRIND true > /dev/null 2>&1; then
>> opt_valgrind_tests=yes
>> - VALGRIND="$VALGRIND -q"
>> + VALGRIND="libtool --mode=execute $VALGRIND \$(VALGRIND_MEMLEAK_OPTS)"
>>
> This needs to be conditioned on whether libtool is used, and shouldn't
> it call the libtool created in the top-level directory of the project
> rather than the system libtool?
>
>
Yes, good point. I didn't think of projects without libtool and version
differences.
Changed in the below patch
Bernd
diff --git a/m4/valgrind-tests.m4 b/m4/valgrind-tests.m4
index e2434c6..a9df59a 100644
--- a/m4/valgrind-tests.m4
+++ b/m4/valgrind-tests.m4
@@ -6,6 +6,53 @@ dnl with or without modifications, as long as this
notice is preserved.
dnl From Simon Josefsson
+AC_ARG_VAR([VALGRIND_MEMLEAK_OPTS],
+ [use this variable to override command line options of
valgrind for memory leak checking. Run 'valgrind --help' or 'man
valgrind' for valid options])
+
+# gl_VALGRIND_TREAT_LEAKS_AS_ERRORS()
+# -----------------------------------
+# Configuration parameter to treat memory leaks as errors if they
+# occur in a module test
+AC_DEFUN([gl_VALGRIND_MEMLEAKS_ARE_ERRORS],
+[
+ AC_ARG_ENABLE(valgrind-memleaks-are-errors,
+ AS_HELP_STRING([--enable-valgrind-memleaks-are-errors],
+ [treat module tests with memory leaks as errors
[default=yes]]),
+ [opt_valgrind_memleaks_are_errors=$enableval],
+ [opt_valgrind_memleaks_are_errors=yes])
+
+ if test -z "$VALGRIND_MEMLEAK_OPTS"; then
+ if test "$opt_valgrind_memleaks_are_errors" = "yes"; then
+ opt_valgrind_memleaks_are_errors=yes
+ VALGRIND_MEMLEAK_OPTS="-q --error-exitcode=1 --leak-check=full"
+ else
+ opt_valgrind_memleaks_are_errors=no
+ VALGRIND_MEMLEAK_OPTS="-q"
+ fi
+ else
+ opt_valgrind_memleaks_are_errors="$VALGRIND_MEMLEAK_OPTS"
+ fi
+
+ AC_SUBST([VALGRIND_MEMLEAK_OPTS],[$VALGRIND_MEMLEAK_OPTS])
+ AC_MSG_CHECKING([whether valgrind treats memory leaks as errors])
+ AC_MSG_RESULT($opt_valgrind_memleaks_are_errors)
+])
+
+
+AC_DEFUN([gl_VALGRIND_SUMMARY],
+[
+ valgrind_options=""
+ if test "$opt_valgrind_tests" = "yes"; then
+ if test "$opt_valgrind_memleaks_are_errors" = "yes" ||
+ test "$opt_valgrind_memleaks_are_errors" = "no";
+ then
+ valgrind_summary="Memleaks are errors:
${opt_valgrind_memleaks_are_errors}"
+ else
+ valgrind_summary="Valgrind options:
${opt_valgrind_memleaks_are_errors}"
+ fi
+ fi
+])
+
# gl_VALGRIND_TESTS()
# -------------------
# Check if valgrind is available, and set VALGRIND to it if available.
@@ -13,7 +60,7 @@ AC_DEFUN([gl_VALGRIND_TESTS],
[
AC_ARG_ENABLE(valgrind-tests,
AS_HELP_STRING([--enable-valgrind-tests],
- [run self tests under valgrind]),
+ [run self tests under valgrind [default=yes]]),
[opt_valgrind_tests=$enableval], [opt_valgrind_tests=yes])
# Run self-tests under valgrind?
@@ -21,14 +68,27 @@ AC_DEFUN([gl_VALGRIND_TESTS],
AC_CHECK_PROGS(VALGRIND, valgrind)
fi
- if test -n "$VALGRIND" && $VALGRIND -q true > /dev/null 2>&1; then
+ if test -n "$VALGRIND" && $VALGRIND true > /dev/null 2>&1; then
opt_valgrind_tests=yes
- VALGRIND="$VALGRIND -q"
+ if test -n "$LIBTOOL"; then
+ VALGRIND="$LIBTOOL --mode=execute $VALGRIND
\$(VALGRIND_MEMLEAK_OPTS)"
+ else
+ VALGRIND="$VALGRIND \$(VALGRIND_MEMLEAK_OPTS)"
+ fi
else
opt_valgrind_tests=no
VALGRIND=
fi
+ gl_VALGRIND_MEMLEAKS_ARE_ERRORS
+ gl_VALGRIND_SUMMARY
+
AC_MSG_CHECKING([whether self tests are run under valgrind])
AC_MSG_RESULT($opt_valgrind_tests)
])
+
+
+
+
+
+