meson: merge our two valgrind configuration conditions into one

Most of the support for valgrind was under HAVE_VALGRIND_VALGRIND_H, i.e. we
would enable if the valgrind headers were found. The operations then we be
conditionalized on RUNNING_UNDER_VALGRIND.

But in a few places we had code which was conditionalized on VALGRIND, i.e. the
config option. I noticed because I compiled with -Dvalgrind=true on a machine
that didn't have valgrind.h, and the build failed because
RUNNING_UNDER_VALGRIND was not defined. My first idea was to add a check that
the header is present if the option is set, but it seems better to just remove
the option. The code to support valgrind is trivial, and if we're
!RUNNING_UNDER_VALGRIND, it has negligible cost. And the case of running under
valgrind is always some special testing/debugging mode, so we should just do
those extra steps to make valgrind output cleaner. Removing the option makes
things simpler and we don't have to think if something should be covered by the
one or the other configuration bit.

I had a vague recollection that in some places we used -Dvalgrind=true not
for valgrind support, but to enable additional cleanup under other sanitizers.
But that code would fail to build without the valgrind headers anyway, so
I'm not sure if that was still used. If there are uses like that, we can
extend the condition for cleanup_pools().

diff --git a/README b/README
index 857cb38bf8cc76ae3b407dabb08884ab87f7346f..97338a633d1b02b1e15b699f5f8242a7a13751a1 100644 (file)
--- a/README
+++ b/README
@@ -403,12 +403,12 @@ WARNINGS and TAINT FLAGS:
         See org.freedesktop.systemd1(5) for more information.
 
 VALGRIND:
-        To run systemd under valgrind, compile with meson option
-        -Dvalgrind=true and have valgrind development headers installed
-        (i.e. valgrind-devel or equivalent). Otherwise, false positives will be
-        triggered by code which violates some rules but is actually safe. Note
-        that valgrind generates nice output only on exit(), hence on shutdown
-        we don't execve() systemd-shutdown.
+        To run systemd under valgrind, compile systemd with the valgrind
+        development headers available (i.e. valgrind-devel or equivalent).
+        Otherwise, false positives will be triggered by code which violates
+        some rules but is actually safe. Note that valgrind generates nice
+        output only on exit(), hence on shutdown we don't execve()
+        systemd-shutdown.
 
 STABLE BRANCHES AND BACKPORTS:
         Stable branches with backported patches are available in the

diff --git a/meson.build b/meson.build
index 319246c6393eb3144454ca9cd7d20e88898b83db..8e37a06b310fbe9f75fff61ffb2895a9f89009b5 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -1020,8 +1020,6 @@ endforeach
 conf.set10('ENABLE_DEBUG_HASHMAP', enable_debug_hashmap)
 conf.set10('ENABLE_DEBUG_MMAP_CACHE', enable_debug_mmap_cache)
 conf.set10('ENABLE_DEBUG_SIPHASH', enable_debug_siphash)
-
-conf.set10('VALGRIND', get_option('valgrind'))
 conf.set10('LOG_TRACE', get_option('log-trace'))
 
 default_user_path = get_option('user-path')
@@ -4608,7 +4606,6 @@ foreach tuple : [
         ['debug hashmap'],
         ['debug mmap cache'],
         ['debug siphash'],
-        ['valgrind',               conf.get('VALGRIND') == 1],
         ['trace logging',          conf.get('LOG_TRACE') == 1],
         ['install tests',          install_tests],
         ['link-udev-shared',       get_option('link-udev-shared')],

diff --git a/meson_options.txt b/meson_options.txt
index f59c3997951d37ae92b5ba1a25d35a6d70f9b802..95b1162249cb3dff4ae947811624fec903250c80 100644 (file)
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -79,8 +79,6 @@ option('bump-proc-sys-fs-file-max', type : 'boolean',
        description : 'bump /proc/sys/fs/file-max to LONG_MAX')
 option('bump-proc-sys-fs-nr-open', type : 'boolean',
        description : 'bump /proc/sys/fs/nr_open to INT_MAX')
-option('valgrind', type : 'boolean', value : false,
-       description : 'do extra operations to avoid valgrind warnings')
 option('log-trace', type : 'boolean', value : false,
        description : 'enable low level debug logging')
 option('user-path', type : 'string',

diff --git a/src/basic/hashmap.c b/src/basic/hashmap.c
index 3d6d99e6dede8af6922738856a5af3b5b574ba60..4b8daf4a29c1f081dede9ab236dd62dc7b5f2cae 100644 (file)
--- a/src/basic/hashmap.c
+++ b/src/basic/hashmap.c
@@ -5,6 +5,9 @@
 #include <pthread.h>
 #include <stdint.h>
 #include <stdlib.h>
+#if HAVE_VALGRIND_VALGRIND_H
+#  include <valgrind/valgrind.h>
+#endif
 
 #include "alloc-util.h"
 #include "fileio.h"
@@ -295,10 +298,11 @@ void hashmap_trim_pools(void) {
         mempool_trim(&ordered_hashmap_pool);
 }
 
-#if VALGRIND
+#if HAVE_VALGRIND_VALGRIND_H
 _destructor_ static void cleanup_pools(void) {
         /* Be nice to valgrind */
-        hashmap_trim_pools();
+        if (RUNNING_ON_VALGRIND)
+                hashmap_trim_pools();
 }
 #endif
 

diff --git a/src/core/main.c b/src/core/main.c
index 1af9b8b50504ecb9acb11f134335499ccf49daab..c9849d05c1ac54cb1ba9bdc670c5619a478a53ab 100644 (file)
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -12,7 +12,7 @@
 #include <seccomp.h>
 #endif
 #if HAVE_VALGRIND_VALGRIND_H
-#include <valgrind/valgrind.h>
+#  include <valgrind/valgrind.h>
 #endif
 
 #include "sd-bus.h"
@@ -1866,8 +1866,8 @@ static int do_reexecute(
                 assert(i <= args_size);
 
                 /*
-                 * We want valgrind to print its memory usage summary before reexecution.  Valgrind won't do
-                 * this is on its own on exec(), but it will do it on exit().  Hence, to ensure we get a
+                 * We want valgrind to print its memory usage summary before reexecution. Valgrind won't do
+                 * this is on its own on exec(), but it will do it on exit(). Hence, to ensure we get a
                  * summary here, fork() off a child, let it exit() cleanly, so that it prints the summary,
                  * and wait() for it in the parent, before proceeding into the exec().
                  */

diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c
index 863649f6df5e77e7c0cb88ac1e4ec3cc9227fc71..e4354199e10b49851a7bf6408cd6e1711426bb9a 100644 (file)
--- a/src/libsystemd-network/test-dhcp-client.c
+++ b/src/libsystemd-network/test-dhcp-client.c
@@ -9,6 +9,9 @@
 #include <stdio.h>
 #include <sys/socket.h>
 #include <unistd.h>
+#if HAVE_VALGRIND_VALGRIND_H
+#  include <valgrind/valgrind.h>
+#endif
 
 #include "sd-dhcp-client.h"
 #include "sd-event.h"
@@ -546,11 +549,12 @@ int main(int argc, char *argv[]) {
         test_discover_message(e);
         test_addr_acq(e);
 
-#if VALGRIND
+#if HAVE_VALGRIND_VALGRIND_H
         /* Make sure the async_close thread has finished.
          * valgrind would report some of the phread_* structures
          * as not cleaned up properly. */
-        sleep(1);
+        if (RUNNING_ON_VALGRIND)
+                sleep(1);
 #endif
 
         return 0;

diff --git a/src/libsystemd/sd-journal/journal-send.c b/src/libsystemd/sd-journal/journal-send.c
index 3b74d2246e2a2e267f010f92b1a440896ae396af..d0d29818c2fc2f0cefb03017169584cd244518b3 100644 (file)
--- a/src/libsystemd/sd-journal/journal-send.c
+++ b/src/libsystemd/sd-journal/journal-send.c
@@ -7,7 +7,7 @@
 #include <sys/un.h>
 #include <unistd.h>
 #if HAVE_VALGRIND_VALGRIND_H
-#include <valgrind/valgrind.h>
+#  include <valgrind/valgrind.h>
 #endif
 
 #define SD_JOURNAL_SUPPRESS_LOCATION
@@ -77,9 +77,9 @@ int journal_fd_nonblock(bool nonblock) {
         return fd_nonblock(r, nonblock);
 }
 
-#if VALGRIND
 void close_journal_fd(void) {
-        /* Be nice to valgrind. This is not atomic. This must be used only in tests. */
+#if HAVE_VALGRIND_VALGRIND_H
+        /* Be nice to valgrind. This is not atomic, so it is useful mainly for debugging. */
 
         if (!RUNNING_ON_VALGRIND)
                 return;
@@ -92,8 +92,8 @@ void close_journal_fd(void) {
 
         safe_close(fd_plus_one - 1);
         fd_plus_one = 0;
-}
 #endif
+}
 
 _public_ int sd_journal_print(int priority, const char *format, ...) {
         int r;

diff --git a/src/libsystemd/sd-journal/journal-send.h b/src/libsystemd/sd-journal/journal-send.h
index 558d39a8c08236a46a8c95b3aaa98090a3571614..24315e249b32d9d0f7a88189d487dd98ac0d0305 100644 (file)
--- a/src/libsystemd/sd-journal/journal-send.h
+++ b/src/libsystemd/sd-journal/journal-send.h
@@ -4,9 +4,4 @@
 #include <stdbool.h>
 
 int journal_fd_nonblock(bool nonblock);
-
-#if VALGRIND
 void close_journal_fd(void);
-#else
-static inline void close_journal_fd(void) {}
-#endif

diff --git a/src/libsystemd/sd-journal/lookup3.c b/src/libsystemd/sd-journal/lookup3.c
index 39967f21cdcecf3e2645921c7ad802a976143fc9..a6a32e09c5bd24001f6b770162573774831730b2 100644 (file)
--- a/src/libsystemd/sd-journal/lookup3.c
+++ b/src/libsystemd/sd-journal/lookup3.c
@@ -320,7 +320,9 @@ uint32_t jenkins_hashlittle( const void *key, size_t length, uint32_t initval)
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
+#define VALGRIND_LIKE (HAVE_VALGRIND_VALGRIND_H || HAS_FEATURE_ADDRESS_SANITIZER || HAS_FEATURE_MEMORY_SANITIZER)
+
+#if !VALGRIND_LIKE
 
     switch(length)
     {
@@ -505,7 +507,7 @@ void jenkins_hashlittle2(
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
+#if !VALGRIND_LIKE
 
     switch(length)
     {
@@ -681,7 +683,7 @@ uint32_t jenkins_hashbig( const void *key, size_t length, uint32_t initval)
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !VALGRIND && !HAS_FEATURE_ADDRESS_SANITIZER && !HAS_FEATURE_MEMORY_SANITIZER
+#if !VALGRIND_LIKE
 
     switch(length)
     {