thread: Convert init/destroy lock functions to use non-const arguments.

'-Wuninitialized-const-pointer' complains about passing uninitialized
data into a function via a const pointer.  That's a valid concern,
since normally the function accepting a const argument wouldn't write
to it, but will likely use the data.

However, we do have some cases of such a coding pattern in OVS today
and they generate a ton of warnings with clang 21.

The main offenders of this are all the types of locks in OVS: mutex,
rwlock, spinlock.  All the locking functions accept const pointers
and then do a CONST_CAST inside.  This is useful for the cases where
we have a lock protecting a structure and want to have some read-only
function that accepts this structure, e.g.:

  struct my_type {
      struct ovs_mutex mutex;
      int value;
  };

  int get_value(const struct my_type *p)
  {
      int res;

      ovs_mutex_lock(&p->mutex);
      res = p->value;
      ovs_mutex_unlock(&p->mutex);

      return res;
  }

If the ovs_mutex_lock() required non-const argument, then we'd have
to use CONST_CAST for every lock/unlock, or we would have to make the
original pointer 'p' non-const, cascading the change through all the
callers of this "read-only" functions.

However, it's totally reasonable for the mutex init and destroy to
not have a constant argument, since we're explicitly trying to change
it and the initialization and destruction likely not happening for
the constant parent structure.

Let's convert both init and destroy API for all the locks to accept
non-const arguments.  This makes clang happy as we're no longer
passing in the uninitialized locks as const.

This technically changes the API, but we're relaxing the restriction,
so it shouldn't be a concern.  All the code that used these functions
before should still work fine.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

diff --git a/include/openvswitch/thread.h b/include/openvswitch/thread.h
index acc822904847a3bc5146932477ccf6d7225fa306..116356d85ae071cf54521dcb7d9f23ff146daa92 100644 (file)
--- a/include/openvswitch/thread.h
+++ b/include/openvswitch/thread.h
@@ -60,10 +60,10 @@ struct OVS_LOCKABLE ovs_spin {
  * Most of these functions abort the process with an error message on any
  * error.  ovs_mutex_trylock() is an exception: it passes through a 0 or EBUSY
  * return value to the caller and aborts on any other error. */
-void ovs_mutex_init(const struct ovs_mutex *);
-void ovs_mutex_init_recursive(const struct ovs_mutex *);
-void ovs_mutex_init_adaptive(const struct ovs_mutex *);
-void ovs_mutex_destroy(const struct ovs_mutex *);
+void ovs_mutex_init(struct ovs_mutex *);
+void ovs_mutex_init_recursive(struct ovs_mutex *);
+void ovs_mutex_init_adaptive(struct ovs_mutex *);
+void ovs_mutex_destroy(struct ovs_mutex *);
 void ovs_mutex_unlock(const struct ovs_mutex *mutex) OVS_RELEASES(mutex);
 void ovs_mutex_lock_at(const struct ovs_mutex *mutex, const char *where)
     OVS_ACQUIRES(mutex);
@@ -79,8 +79,8 @@ void ovs_mutex_cond_wait(pthread_cond_t *, const struct ovs_mutex *mutex)
     OVS_REQUIRES(mutex);
 
 #ifdef HAVE_PTHREAD_SPIN_LOCK
-void ovs_spin_init(const struct ovs_spin *);
-void ovs_spin_destroy(const struct ovs_spin *);
+void ovs_spin_init(struct ovs_spin *);
+void ovs_spin_destroy(struct ovs_spin *);
 void ovs_spin_unlock(const struct ovs_spin *spin) OVS_RELEASES(spin);
 void ovs_spin_lock_at(const struct ovs_spin *spin, const char *where)
     OVS_ACQUIRES(spin);

diff --git a/lib/ovs-thread.c b/lib/ovs-thread.c
index f80008061562f3ca86de9ac4ad9c07518233b46e..243791de8a2600e58409288d73f61c22c5af0f15 100644 (file)
--- a/lib/ovs-thread.c
+++ b/lib/ovs-thread.c
@@ -113,9 +113,9 @@ TRY_LOCK_FUNCTION(rwlock, trywrlock);
 TRY_LOCK_FUNCTION(spin, trylock);
 #endif
 
-#define UNLOCK_FUNCTION(TYPE, FUN, WHERE) \
+#define UNLOCK_FUNCTION(TYPE, FUN, WHERE, CONST) \
     void \
-    ovs_##TYPE##_##FUN(const struct ovs_##TYPE *l_) \
+    ovs_##TYPE##_##FUN(CONST struct ovs_##TYPE *l_) \
         OVS_NO_THREAD_SAFETY_ANALYSIS \
     { \
         struct ovs_##TYPE *l = CONST_CAST(struct ovs_##TYPE *, l_); \
@@ -131,13 +131,13 @@ TRY_LOCK_FUNCTION(spin, trylock);
                        ovs_strerror(error)); \
         } \
     }
-UNLOCK_FUNCTION(mutex, unlock, "<unlocked>");
-UNLOCK_FUNCTION(mutex, destroy, NULL);
-UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>");
-UNLOCK_FUNCTION(rwlock, destroy, NULL);
+UNLOCK_FUNCTION(mutex, unlock, "<unlocked>", const);
+UNLOCK_FUNCTION(mutex, destroy, NULL, /* non-const */);
+UNLOCK_FUNCTION(rwlock, unlock, "<unlocked>", const);
+UNLOCK_FUNCTION(rwlock, destroy, NULL, /* non-const */);
 #ifdef HAVE_PTHREAD_SPIN_LOCK
-UNLOCK_FUNCTION(spin, unlock, "<unlocked>");
-UNLOCK_FUNCTION(spin, destroy, NULL);
+UNLOCK_FUNCTION(spin, unlock, "<unlocked>", const);
+UNLOCK_FUNCTION(spin, destroy, NULL, /* non-const */);
 #endif
 
 #define XPTHREAD_FUNC1(FUNCTION, PARAM1)                \
@@ -199,9 +199,8 @@ XPTHREAD_FUNC3(pthread_sigmask, int, const sigset_t *, sigset_t *);
 #endif
 
 static void
-ovs_mutex_init__(const struct ovs_mutex *l_, int type)
+ovs_mutex_init__(struct ovs_mutex *l, int type)
 {
-    struct ovs_mutex *l = CONST_CAST(struct ovs_mutex *, l_);
     pthread_mutexattr_t attr;
     int error;
 
@@ -217,21 +216,21 @@ ovs_mutex_init__(const struct ovs_mutex *l_, int type)
 
 /* Initializes 'mutex' as a normal (non-recursive) mutex. */
 void
-ovs_mutex_init(const struct ovs_mutex *mutex)
+ovs_mutex_init(struct ovs_mutex *mutex)
 {
     ovs_mutex_init__(mutex, PTHREAD_MUTEX_ERRORCHECK);
 }
 
 /* Initializes 'mutex' as a recursive mutex. */
 void
-ovs_mutex_init_recursive(const struct ovs_mutex *mutex)
+ovs_mutex_init_recursive(struct ovs_mutex *mutex)
 {
     ovs_mutex_init__(mutex, PTHREAD_MUTEX_RECURSIVE);
 }
 
 /* Initializes 'mutex' as a recursive mutex. */
 void
-ovs_mutex_init_adaptive(const struct ovs_mutex *mutex)
+ovs_mutex_init_adaptive(struct ovs_mutex *mutex)
 {
 #ifdef PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP
     ovs_mutex_init__(mutex, PTHREAD_MUTEX_ADAPTIVE_NP);
@@ -241,9 +240,8 @@ ovs_mutex_init_adaptive(const struct ovs_mutex *mutex)
 }
 
 void
-ovs_rwlock_init(const struct ovs_rwlock *l_)
+ovs_rwlock_init(struct ovs_rwlock *l)
 {
-    struct ovs_rwlock *l = CONST_CAST(struct ovs_rwlock *, l_);
     int error;
 
     l->where = "<unlocked>";
@@ -287,9 +285,8 @@ ovs_mutex_cond_wait(pthread_cond_t *cond, const struct ovs_mutex *mutex_)
 
 #ifdef HAVE_PTHREAD_SPIN_LOCK
 static void
-ovs_spin_init__(const struct ovs_spin *l_, int pshared)
+ovs_spin_init__(struct ovs_spin *l, int pshared)
 {
-    struct ovs_spin *l = CONST_CAST(struct ovs_spin *, l_);
     int error;
 
     l->where = "<unlocked>";
@@ -300,7 +297,7 @@ ovs_spin_init__(const struct ovs_spin *l_, int pshared)
 }
 
 void
-ovs_spin_init(const struct ovs_spin *spin)
+ovs_spin_init(struct ovs_spin *spin)
 {
     ovs_spin_init__(spin, PTHREAD_PROCESS_PRIVATE);
 }

diff --git a/lib/ovs-thread.h b/lib/ovs-thread.h
index aac5e19c99c8b986271f10e1c5730aced675b0ae..d37f4ffc4519acd5362df99fe8f5adb70c003a44 100644 (file)
--- a/lib/ovs-thread.h
+++ b/lib/ovs-thread.h
@@ -69,8 +69,8 @@ struct OVS_LOCKABLE ovs_rwlock {
  * Most of these functions abort the process with an error message on any
  * error.  The "trylock" functions are exception: they pass through a 0 or
  * EBUSY return value to the caller and abort on any other error. */
-void ovs_rwlock_init(const struct ovs_rwlock *);
-void ovs_rwlock_destroy(const struct ovs_rwlock *);
+void ovs_rwlock_init(struct ovs_rwlock *);
+void ovs_rwlock_destroy(struct ovs_rwlock *);
 void ovs_rwlock_unlock(const struct ovs_rwlock *rwlock) OVS_RELEASES(rwlock);
 
 /* Wrappers for pthread_rwlockattr_*() that abort the process on any error. */