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

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

[Chen Qi]

The following changes since commit 33b7885ecdc8774e34ac3534ec49fed6ffdb3916:

  oprofile: 0.9.9 -> 1.0.0 (2014-11-09 10:19:58 +0000)

are available in the git repository at:

  git://git.openembedded.org/openembedded-core-contrib ChenQi/systemd-valgrindheader
  http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=ChenQi/systemd-valgrindheader

Chen Qi (1):
  systemd: add option for valgrind header checking

 ...re.ac-add-valgrind-header-checking-option.patch | 28 ++++++++++++++++++++++
 meta/recipes-core/systemd/systemd_216.bb           |  2 ++
 2 files changed, 30 insertions(+)
 create mode 100644 meta/recipes-core/systemd/systemd/0001-configure.ac-add-valgrind-header-checking-option.patch

-- 
1.9.1

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

[Chen Qi]

Add option so that valgrind header checking is controlled. Otherwise,
we would get the following error.

   src/libsystemd/sd-bus/bus-control.c: fatal error: valgrind/mencheck.h: No such file or directory

The above error could be reproduced by the following command.

bitbake valgrind && bitbake systemd -ccleansstate && bitbake systemd -cconfigure
&& bitbake valgrind -ccleansstate && bitbake systemd -ccompile

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 ...re.ac-add-valgrind-header-checking-option.patch | 28 ++++++++++++++++++++++
 meta/recipes-core/systemd/systemd_216.bb           |  2 ++
 2 files changed, 30 insertions(+)
 create mode 100644 meta/recipes-core/systemd/systemd/0001-configure.ac-add-valgrind-header-checking-option.patch

diff --git a/meta/recipes-core/systemd/systemd/0001-configure.ac-add-valgrind-header-checking-option.patch b/meta/recipes-core/systemd/systemd/0001-configure.ac-add-valgrind-header-checking-option.patch
new file mode 100644
index 0000000..89f3348
--- /dev/null
+++ b/meta/recipes-core/systemd/systemd/0001-configure.ac-add-valgrind-header-checking-option.patch
@@ -0,0 +1,28 @@
+Upstream-Status: Pending
+
+Subject: configure.ac: add valgrind header checking option
+
+Signed-off-by: Chen qi <Qi.Chen@windriver.com>
+---
+ configure.ac | 5 ++++-
+ 1 file changed, 4 insertions(+), 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index 945adfc..dd4d7b2 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -1141,7 +1141,10 @@ AC_DEFINE_UNQUOTED(TELINIT, ["$TELINIT"], [Path to telinit])
+ 
+ AC_SUBST(TELINIT)
+ 
+-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
+ 
+ # ------------------------------------------------------------------------------
+ have_myhostname=no
+-- 
+1.9.1
+
diff --git a/meta/recipes-core/systemd/systemd_216.bb b/meta/recipes-core/systemd/systemd_216.bb
index 27a5d92..abef45d 100644
--- a/meta/recipes-core/systemd/systemd_216.bb
+++ b/meta/recipes-core/systemd/systemd_216.bb
@@ -38,6 +38,7 @@ SRC_URI = "git://anongit.freedesktop.org/systemd/systemd;branch=master;protocol=
            file://init \
            file://run-ptest \
            ${@bb.utils.contains('PACKAGECONFIG', 'resolved', '', 'file://0001-tmpfiles.d-etc.conf-disable-resolv.conf-symlink.patch', d)} \
+           file://0001-configure.ac-add-valgrind-header-checking-option.patch \
           "
 
 S = "${WORKDIR}/git"
@@ -64,6 +65,7 @@ PACKAGECONFIG[elfutils] = "--enable-elfutils,--disable-elfutils,elfutils"
 PACKAGECONFIG[resolved] = "--enable-resolved,--disable-resolved"
 PACKAGECONFIG[networkd] = "--enable-networkd,--disable-networkd"
 PACKAGECONFIG[libidn] = "--enable-libidn,--disable-libidn,libidn"
+PACKAGECONFIG[valgrindheader] = "--enable-valgrindheader,--disable-valgrindheader"
 
 CACHED_CONFIGUREVARS = "ac_cv_path_KILL=${base_bindir}/kill"
 
-- 
1.9.1

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

[Burton, Ross]

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

On 12 November 2014 10:45, Chen Qi <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.

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: 1645 bytes --]

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

[ChenQi]

[-- 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 --]