Fix: waiter: use std::reference_wrapper instead of a raw reference
authorJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 22 Mar 2024 15:46:36 +0000 (11:46 -0400)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Fri, 22 Mar 2024 18:25:11 +0000 (14:25 -0400)
clang-tidy warns that
  /home/jgalar/EfficiOS/src/lttng-tools/src/common/waiter.hpp:32:9: warning: use '= default' to define a trivial copy-assignment operator [modernize-use-equals-default]

The warning itself is bogus since it is not possible to use the default
copy-assignment operator in this case. Indeed, _state being a reference,
it cannot be rebound after its initialization. The compiler will refuse
to generate a default implementation as it would be ill-formed.

However, this does highlight a problem in the implementation, namely
that the explicit assignment operator does attempt to re-assign _state.

Switching the _state reference to use std::reference_wrapper does make
it legal to re-bind the reference to a different object.

Having the ability to assign a waker is conceptually a bit strange (vs
obtaining another instance from the waiter) and should probably be
disabled, but it is required for the moment as one the of use in a huge
C structure that gets copied using by assignment.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I6f4c740244decb50426ec0571aaea754edaea6a5

src/common/waiter.cpp
src/common/waiter.hpp

index 1cfd3e6f7778f14a630b4e34ff69bf6a6081214f..8d2ba4d696ec228a5f886bc79ef10fa29f34c36e 100644 (file)
@@ -122,18 +122,18 @@ void lttng::synchro::waker::wake()
 {
        cmm_smp_mb();
 
-       LTTNG_ASSERT(uatomic_read(&_state) == WAITER_WAITING);
+       LTTNG_ASSERT(uatomic_read(&_state.get()) == WAITER_WAITING);
 
-       uatomic_set(&_state, WAITER_WOKEN_UP);
-       if (!(uatomic_read(&_state) & WAITER_RUNNING)) {
-               if (futex_noasync(&_state, FUTEX_WAKE, 1, nullptr, nullptr, 0) < 0) {
+       uatomic_set(&_state.get(), WAITER_WOKEN_UP);
+       if (!(uatomic_read(&_state.get()) & WAITER_RUNNING)) {
+               if (futex_noasync(&_state.get(), FUTEX_WAKE, 1, nullptr, nullptr, 0) < 0) {
                        PERROR("futex_noasync");
                        abort();
                }
        }
 
        /* Allow teardown of struct urcu_wait memory. */
-       uatomic_or(&_state, WAITER_TEARDOWN);
+       uatomic_or(&_state.get(), WAITER_TEARDOWN);
 }
 
 lttng::synchro::wait_queue::wait_queue()
index 7f5c2f171f4e10448e011cff42a0cc7022d05a85..09823dd4f94a6ec3c74032abc86be23cace3b48d 100644 (file)
@@ -14,7 +14,7 @@
 
 #include "macros.hpp"
 
-#include <stdbool.h>
+#include <functional>
 #include <stdint.h>
 #include <urcu/wfstack.h>
 
@@ -29,16 +29,8 @@ class waker {
 public:
        waker(const waker&) = default;
        waker(waker&&) = default;
-       waker& operator=(const waker& other)
-       {
-               _state = other._state;
-               return *this;
-       }
-       waker& operator=(waker&& other)
-       {
-               _state = other._state;
-               return *this;
-       }
+       waker& operator=(const waker& other) = default;
+       waker& operator=(waker&& other) = default;
 
        void wake();
 
@@ -49,7 +41,7 @@ private:
        {
        }
 
-       int32_t& _state;
+       std::reference_wrapper<int32_t> _state;
 };
 
 class waiter final {
This page took 0.026749 seconds and 4 git commands to generate.