bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] suggestion enhancement valgrind_tests.m4


From: Simon Josefsson
Subject: Re: [PATCH] suggestion enhancement valgrind_tests.m4
Date: Tue, 16 Nov 2010 22:51:06 +0100
User-agent: Gnus/5.110011 (No Gnus v0.11) Emacs/23.2 (gnu/linux)

Bernd Becker <address@hidden> writes:

> Hi,
>
> I tried this a few weeks ago, but didn't get a response, yet.
> So maybe this time I have more luck

Sorry for slow response!  Thanks for working on this.  If you want the
patch to go into gnulib, you need to sign over the copyright to the FSF.
Have you done this?  I can send you papers, e-mail me off the list.

Quick review of the patch below.

> +# 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]),
> +    [opt_valgrind_memleaks_are_errors=$enableval],
> +    [opt_valgrind_memleaks_are_errors=no])
> +
> +  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)
> +])

This seems fine, I didn't know about the --error-exitcode=1
--leak-check=full parameters.  That will help in self-tests.

> ++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.

> +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?

> +
>  # 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?

>
>    # 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?

/Simon



reply via email to

[Prev in Thread] Current Thread [Next in Thread]