From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Fri, 22 Mar 2024 15:46:36 +0000 (-0400)
Subject: Fix: waiter: use std::reference_wrapper instead of a raw reference
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=ac8791840c1815f10336210b4c0d7046eb515692;p=lttng-tools.git

Fix: waiter: use std::reference_wrapper instead of a raw reference

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
---

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 <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 {