From ac8791840c1815f10336210b4c0d7046eb515692 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 22 Mar 2024 11:46:36 -0400 Subject: [PATCH] Fix: waiter: use std::reference_wrapper instead of a raw reference MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Change-Id: I6f4c740244decb50426ec0571aaea754edaea6a5 --- src/common/waiter.cpp | 10 +++++----- src/common/waiter.hpp | 16 ++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/common/waiter.cpp b/src/common/waiter.cpp index 1cfd3e6f7..8d2ba4d69 100644 --- a/src/common/waiter.cpp +++ b/src/common/waiter.cpp @@ -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() diff --git a/src/common/waiter.hpp b/src/common/waiter.hpp index 7f5c2f171..09823dd4f 100644 --- a/src/common/waiter.hpp +++ b/src/common/waiter.hpp @@ -14,7 +14,7 @@ #include "macros.hpp" -#include +#include #include #include @@ -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 _state; }; class waiter final { -- 2.34.1