Turn VALGRIND variable into a meson configuration switch

Configuration through environment variable is inconvenient with meson, because
they cannot be convieniently changed and/or are not preserved during
reconfiguration (https://github.com/mesonbuild/meson/issues/1503).
This adds -Dvalgrind=true/false, which has the advantage that it can be set
at any time with meson configure -Dvalgrind=... and ninja will rebuild targets
as necessary. Additional minor advantages are better consistency with the
options for hashmap debugging, and typo avoidance with '#if' instead of '#ifdef'.

README

@@ -303,13 +303,12 @@ WARNINGS:
         For more information on this issue consult
         https://www.freedesktop.org/wiki/Software/systemd/separate-usr-is-broken
 
-        To run systemd under valgrind, compile with VALGRIND defined
+        To run systemd under valgrind, compile with meson option
-        (e.g. CPPFLAGS='... -DVALGRIND=1' meson <options>) and have valgrind
+        -Dvalgrind=true and have valgrind development headers installed
-        development headers installed (i.e. valgrind-devel or
+        (i.e. valgrind-devel or equivalent). Otherwise, false positives will be
-        equivalent). Otherwise, false positives will be triggered by code which
+        triggered by code which violates some rules but is actually safe. Note
-        violates some rules but is actually safe. Note that valgrind generates
+        that valgrind generates nice output only on exit(), hence on shutdown
-        nice output only on exit(), hence on shutdown we don't execve()
+        we don't execve() systemd-shutdown.
-        systemd-shutdown.
 
 STABLE BRANCHES AND BACKPORTS
 

meson.build

@@ -774,6 +774,8 @@ endforeach
 conf.set10('ENABLE_DEBUG_HASHMAP', enable_debug_hashmap)
 conf.set10('ENABLE_DEBUG_MMAP_CACHE', enable_debug_mmap_cache)
 
+conf.set10('VALGRIND', get_option('valgrind'))
+
 #####################################################################
 
 threads = dependency('threads')
@@ -2925,6 +2927,7 @@ foreach tuple : [
         ['gshadow'],
         ['debug hashmap'],
         ['debug mmap cache'],
+        ['valgrind',         conf.get('VALGRIND') == 1],
 ]
 
         if tuple.length() >= 2

meson_options.txt

@@ -50,6 +50,8 @@ option('debug', type : 'array', choices : ['hashmap', 'mmap-cache'], value : [],
        description : 'enable extra debugging')
 option('memory-accounting-default', type : 'boolean',
        description : 'enable MemoryAccounting= by default')
+option('valgrind', type : 'boolean', value : false,
+       description : 'do extra operations to avoid valgrind warnings')
 
 option('utmp', type : 'boolean',
        description : 'support for utmp/wtmp log handling')

src/basic/hashmap.c

@@ -281,7 +281,7 @@ static const struct hashmap_type_info hashmap_type_info[_HASHMAP_TYPE_MAX] = {
         },
 };
 
-#ifdef VALGRIND
+#if VALGRIND
 __attribute__((destructor)) static void cleanup_pools(void) {
         _cleanup_free_ char *t = NULL;
         int r;

src/basic/mempool.c

@@ -76,7 +76,7 @@ void mempool_free_tile(struct mempool *mp, void *p) {
         mp->freelist = p;
 }
 
-#ifdef VALGRIND
+#if VALGRIND
 
 void mempool_drop(struct mempool *mp) {
         struct pool *p = mp->first_pool;

src/basic/mempool.h

@@ -30,6 +30,6 @@ static struct mempool pool_name = { \
 }
 
 
-#ifdef VALGRIND
+#if VALGRIND
 void mempool_drop(struct mempool *mp);
 #endif

src/journal/lookup3.c

@@ -315,11 +315,11 @@ uint32_t jenkins_hashlittle( const void *key, size_t length, uint32_t initval)
      * then masks off the part it's not allowed to read.  Because the
      * string is aligned, the masked-off tail is in the same word as the
      * rest of the string.  Every machine with memory protection I've seen
-     * does it on word boundaries, so is OK with this.  But VALGRIND will
+     * does it on word boundaries, so is OK with this.  But valgrind will
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !defined(VALGRIND) && !defined(__SANITIZE_ADDRESS__)
+#if !VALGRIND && !defined(__SANITIZE_ADDRESS__)
 
     switch(length)
     {
@@ -500,11 +500,11 @@ void jenkins_hashlittle2(
      * then masks off the part it's not allowed to read.  Because the
      * string is aligned, the masked-off tail is in the same word as the
      * rest of the string.  Every machine with memory protection I've seen
-     * does it on word boundaries, so is OK with this.  But VALGRIND will
+     * does it on word boundaries, so is OK with this.  But valgrind will
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !defined(VALGRIND) && !defined(__SANITIZE_ADDRESS__)
+#if !VALGRIND && !defined(__SANITIZE_ADDRESS__)
 
     switch(length)
     {
@@ -676,11 +676,11 @@ uint32_t jenkins_hashbig( const void *key, size_t length, uint32_t initval)
      * then shifts out the part it's not allowed to read.  Because the
      * string is aligned, the illegal read is in the same word as the
      * rest of the string.  Every machine with memory protection I've seen
-     * does it on word boundaries, so is OK with this.  But VALGRIND will
+     * does it on word boundaries, so is OK with this.  But valgrind will
      * still catch it and complain.  The masking trick does make the hash
      * noticeably faster for short strings (like English words).
      */
-#if !defined(VALGRIND) && !defined(__SANITIZE_ADDRESS__)
+#if !VALGRIND && !defined(__SANITIZE_ADDRESS__)
 
     switch(length)
     {

src/libsystemd-network/test-dhcp-client.c

@@ -539,7 +539,7 @@ int main(int argc, char *argv[]) {
         test_discover_message(e);
         test_addr_acq(e);
 
-#ifdef VALGRIND
+#if VALGRIND
         /* Make sure the async_close thread has finished.
          * valgrind would report some of the phread_* structures
          * as not cleaned up properly. */