Re: [PATCH 1/1] systemd: add option for valgrind header checking

[ChenQi <Qi.Chen@windriver.com>]

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

On 11/12/2014 11:55 PM, Burton, Ross wrote:
> On 12 November 2014 10:45, Chen Qi <Qi.Chen@windriver.com 
> <mailto:Qi.Chen@windriver.com>> wrote:
>
>     +-AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h valgrind/valgrind.h])
>     ++AC_ARG_ENABLE(valgrindheader,
>     AS_HELP_STRING([--disable-valgrindheader], [disable valgrind
>     header checking]))
>     ++if test "x$enable_valgrindheader" != "xno"; then
>     ++        AC_CHECK_HEADERS_ONCE([valgrind/memcheck.h
>     valgrind/valgrind.h])
>     ++fi
>
>
> If the user passes --enable-valgrindheader and valgrind isn't present, 
> it doesn't fail.  Combined with the lack of a valgrind build 
> dependency in the PACKAGECONFIG statement this isn't a very useful 
> PACKAGECONFIG option as it only works when not specified, and if it's 
> specified is non-deterministic.
>

Hi Ross,

I've sent to V2 with modification in PACKAGECONFIG. With this 
modification, the build result would not be non-deterministic.

Best Regards,
Chen Qi

> It appears that this valgrind support is for running the test suite 
> under valgrind for leak detection, which is something we're unlikely 
> to do.  A quick fix would be to comment out the check.
>
> A proper fix would be to use AC_ARG_WITH(valgrind), default to "auto", 
> use the pkg-config macros and respect yes/no/auto as appropriate.  The 
> mantra here being to spend ten minutes fixing it properly upstream 
> instead of having to rewrite the patch often as upstream changes their 
> configure script.
>
> Ross


[-- Attachment #2: Type: text/html, Size: 3242 bytes --]