From: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Date: Wed, 17 Jul 2024 20:39:54 +0000 (+0000)
Subject: urcu: add lfht_iteration_adapter helper
X-Git-Url: http://git.lttng.org./?a=commitdiff_plain;h=ef2f8a02adc70fba061f88381a3d477fa78ba3be;p=lttng-tools.git

urcu: add lfht_iteration_adapter helper

The urcu lfht macros often make use of caa_container_of (and other
equivalent variations) which use offsetof. Unfortunately, offsetof is
conditionally supported by compilers for non-POD types.

The tree already has lttng::utils::container_of to work around this
problem. This new utils makes it possible to iterate on all of the
elements of an lfht without using those macros. Those iterations are the
main reason such warnings are emitted. The interface of
lfht_iteration_adapter also allows the use of ranged-for loops.

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

diff --git a/src/bin/lttng-sessiond/save.cpp b/src/bin/lttng-sessiond/save.cpp
index 67fa01de6..b5b91ed7d 100644
--- a/src/bin/lttng-sessiond/save.cpp
+++ b/src/bin/lttng-sessiond/save.cpp
@@ -1195,7 +1195,6 @@ static int save_agent_events(struct config_writer *writer, struct agent *agent)
 			struct agent_event *agent_event;
 			struct ltt_ust_event fake_event;
 
-			memset(&fake_event, 0, sizeof(fake_event));
 			agent_event = lttng::utils::container_of(node, &agent_event::node);
 
 			/*
diff --git a/src/bin/lttng-sessiond/snapshot.cpp b/src/bin/lttng-sessiond/snapshot.cpp
index d4fb5f069..d326e30dd 100644
--- a/src/bin/lttng-sessiond/snapshot.cpp
+++ b/src/bin/lttng-sessiond/snapshot.cpp
@@ -42,8 +42,6 @@ static int output_init(const ltt_session::locked_ref& session,
 {
 	int ret = 0, i;
 
-	memset(output, 0, sizeof(struct snapshot_output));
-
 	/*
 	 * max_size of -1ULL means unset. Set to default (unlimited).
 	 */
@@ -178,7 +176,7 @@ error:
 
 struct snapshot_output *snapshot_output_alloc()
 {
-	return zmalloc<snapshot_output>();
+	return new snapshot_output;
 }
 
 /*
@@ -233,7 +231,8 @@ void snapshot_output_destroy(struct snapshot_output *obj)
 		consumer_output_send_destroy_relayd(obj->consumer);
 		consumer_output_put(obj->consumer);
 	}
-	free(obj);
+
+	delete obj;
 }
 
 /*
diff --git a/src/bin/lttng-sessiond/snapshot.hpp b/src/bin/lttng-sessiond/snapshot.hpp
index bdc520643..4933d190c 100644
--- a/src/bin/lttng-sessiond/snapshot.hpp
+++ b/src/bin/lttng-sessiond/snapshot.hpp
@@ -20,23 +20,23 @@
 struct consumer_output;
 
 struct snapshot_output {
-	uint32_t id;
-	uint64_t max_size;
+	uint32_t id = 0;
+	uint64_t max_size = 0;
 	/* Number of snapshot taken with that output. */
-	uint64_t nb_snapshot;
-	char name[NAME_MAX];
-	struct consumer_output *consumer;
-	int kernel_sockets_copied;
-	int ust_sockets_copied;
+	uint64_t nb_snapshot = 0;
+	char name[NAME_MAX] = {};
+	struct consumer_output *consumer = nullptr;
+	bool kernel_sockets_copied = false;
+	bool ust_sockets_copied = false;
 	/*
 	 * Contains the string with "<date>-<time>" for when the snapshot command
 	 * is triggered. This is to make sure every streams will use the same time
 	 * for the directory output.
 	 */
-	char datetime[16];
+	char datetime[16] = {};
 
 	/* Indexed by ID. */
-	struct lttng_ht_node_ulong node;
+	struct lttng_ht_node_ulong node = {};
 };
 
 struct snapshot {
diff --git a/src/common/hashtable/hashtable.cpp b/src/common/hashtable/hashtable.cpp
index a410fc41c..a4ae7542d 100644
--- a/src/common/hashtable/hashtable.cpp
+++ b/src/common/hashtable/hashtable.cpp
@@ -36,8 +36,8 @@ static unsigned long max_hash_buckets_size = 0;
  */
 static int match_str(struct cds_lfht_node *node, const void *key)
 {
-	struct lttng_ht_node_str *match_node =
-		lttng::utils::container_of(node, &lttng_ht_node_str::node);
+	auto *match_node = reinterpret_cast<lttng_ht_node_str *>(
+		lttng::utils::container_of(node, &lttng_ht_node_str::node));
 
 	return hash_match_key_str(match_node->key, (void *) key);
 }
@@ -47,8 +47,8 @@ static int match_str(struct cds_lfht_node *node, const void *key)
  */
 static int match_ulong(struct cds_lfht_node *node, const void *key)
 {
-	struct lttng_ht_node_ulong *match_node =
-		lttng::utils::container_of(node, &lttng_ht_node_ulong::node);
+	auto *match_node = reinterpret_cast<lttng_ht_node_ulong *>(
+		lttng::utils::container_of(node, &lttng_ht_node_ulong::node));
 
 	return hash_match_key_ulong((void *) match_node->key, (void *) key);
 }
@@ -58,8 +58,8 @@ static int match_ulong(struct cds_lfht_node *node, const void *key)
  */
 static int match_u64(struct cds_lfht_node *node, const void *key)
 {
-	struct lttng_ht_node_u64 *match_node =
-		lttng::utils::container_of(node, &lttng_ht_node_u64::node);
+	auto *match_node = reinterpret_cast<lttng_ht_node_u64 *>(
+		lttng::utils::container_of(node, &lttng_ht_node_u64::node));
 
 	return hash_match_key_u64(&match_node->key, (void *) key);
 }
@@ -69,8 +69,8 @@ static int match_u64(struct cds_lfht_node *node, const void *key)
  */
 static int match_two_u64(struct cds_lfht_node *node, const void *key)
 {
-	struct lttng_ht_node_two_u64 *match_node =
-		lttng::utils::container_of(node, &lttng_ht_node_two_u64::node);
+	auto *match_node = reinterpret_cast<lttng_ht_node_two_u64 *>(
+		lttng::utils::container_of(node, &lttng_ht_node_two_u64::node));
 
 	return hash_match_key_two_u64((void *) &match_node->key, (void *) key);
 }
@@ -405,7 +405,8 @@ struct lttng_ht_node_ulong *lttng_ht_add_replace_ulong(struct lttng_ht *ht,
 	if (!node_ptr) {
 		return nullptr;
 	} else {
-		return lttng::utils::container_of(node_ptr, &lttng_ht_node_ulong::node);
+		return reinterpret_cast<lttng_ht_node_ulong *>(
+			lttng::utils::container_of(node_ptr, &lttng_ht_node_ulong::node));
 	}
 }
 
@@ -431,7 +432,8 @@ struct lttng_ht_node_u64 *lttng_ht_add_replace_u64(struct lttng_ht *ht,
 		return nullptr;
 	} else {
 		LTTNG_ASSERT(node_ptr == &node->node);
-		return lttng::utils::container_of(node_ptr, &lttng_ht_node_u64::node);
+		return reinterpret_cast<lttng_ht_node_u64 *>(
+			lttng::utils::container_of(node_ptr, &lttng_ht_node_u64::node));
 	}
 }
 
diff --git a/src/common/hashtable/hashtable.hpp b/src/common/hashtable/hashtable.hpp
index db258e4d9..515286385 100644
--- a/src/common/hashtable/hashtable.hpp
+++ b/src/common/hashtable/hashtable.hpp
@@ -42,21 +42,22 @@ struct lttng_ht_iter {
 	struct cds_lfht_iter iter;
 };
 
-struct lttng_ht_node_str {
-	char *key;
+struct lttng_ht_node {
 	struct cds_lfht_node node;
+};
+
+struct lttng_ht_node_str : public lttng_ht_node {
+	char *key;
 	struct rcu_head head;
 };
 
-struct lttng_ht_node_ulong {
+struct lttng_ht_node_ulong : public lttng_ht_node {
 	unsigned long key;
-	struct cds_lfht_node node;
 	struct rcu_head head;
 };
 
-struct lttng_ht_node_u64 {
+struct lttng_ht_node_u64 : public lttng_ht_node {
 	uint64_t key;
-	struct cds_lfht_node node;
 	struct rcu_head head;
 };
 
@@ -65,9 +66,8 @@ struct lttng_ht_two_u64 {
 	uint64_t key2;
 };
 
-struct lttng_ht_node_two_u64 {
+struct lttng_ht_node_two_u64 : public lttng_ht_node {
 	struct lttng_ht_two_u64 key;
-	struct cds_lfht_node node;
 	struct rcu_head head;
 };
 
@@ -123,7 +123,20 @@ NodeType *lttng_ht_iter_get_node(const lttng_ht_iter *iter)
 		return nullptr;
 	}
 
-	return lttng::utils::container_of(node, &NodeType::node);
+	return reinterpret_cast<NodeType *>(lttng::utils::container_of(node, &NodeType::node));
+}
+
+template <class ParentType, class NodeType>
+ParentType *lttng_ht_node_container_of(cds_lfht_node *node, const NodeType ParentType::*Member)
+{
+	/*
+	 * The node member is within lttng_ht_node, the parent class of all ht wrapper nodes. We
+	 * compute the address of the ht wrapper node from the native lfht node.
+	 */
+	auto *wrapper_node = lttng::utils::container_of(node, &NodeType::node);
+
+	/* Knowing the address of the wrapper node, we can get that of the contained type. */
+	return lttng::utils::container_of(static_cast<NodeType *>(wrapper_node), Member);
 }
 
 #endif /* _LTT_HT_H */
diff --git a/src/common/urcu.hpp b/src/common/urcu.hpp
index 017c9fad5..44a3abaf1 100644
--- a/src/common/urcu.hpp
+++ b/src/common/urcu.hpp
@@ -9,8 +9,14 @@
 #define LTTNG_URCU_H
 
 #define _LGPL_SOURCE
+#include <common/exception.hpp>
+#include <common/hashtable/hashtable.hpp>
+#include <common/macros.hpp>
+
+#include <iterator>
 #include <mutex>
 #include <urcu.h>
+#include <urcu/rculfhash.h>
 
 namespace lttng {
 namespace urcu {
@@ -75,6 +81,117 @@ private:
 
 using unique_read_lock = std::unique_lock<details::read_lock>;
 
+namespace details {
+
+/* Implementation for types that contain a straight cds_lfht_node. */
+template <typename ContainedType, typename NodeType, NodeType ContainedType::*Member>
+static typename std::enable_if<std::is_same<cds_lfht_node, NodeType>::value, ContainedType *>::type
+get_element_from_node(cds_lfht_node& node)
+{
+	return lttng::utils::container_of(&node, Member);
+}
+
+/* Specialization for NodeType deriving from lttng_ht_node. */
+template <typename ContainedType, typename NodeType, NodeType ContainedType::*Member>
+static typename std::enable_if<std::is_base_of<lttng_ht_node, NodeType>::value, ContainedType *>::type
+get_element_from_node(cds_lfht_node& node)
+{
+	return lttng_ht_node_container_of(&node, Member);
+}
+} /* namespace details */
+
+/*
+ * The lfht_iteration_adapter class template wraps the liburcu lfht API to provide iteration
+ * capabilities. It allows users to iterate over a lock-free hash table with ranged-for semantics
+ * while holding the RCU read lock. The reader lock is held for the lifetime of the iteration
+ * adapter (i.e. not the lifetime of the iterators it provides).
+ */
+template <typename ContainedType, typename NodeType, NodeType ContainedType::*Member>
+class lfht_iteration_adapter {
+public:
+	/* Nested iterator class defines the iterator for lfht_iteration_adapter. */
+	class iterator : public std::iterator<std::input_iterator_tag, std::uint64_t> {
+		/* Allow lfht_iteration_adapter to access private members of iterator_base. */
+		friend lfht_iteration_adapter;
+
+	public:
+		iterator(const iterator& other) = default;
+		iterator(iterator&& other) noexcept = default;
+		~iterator() = default;
+
+		/* Move to the next element in the hash table. */
+		iterator& operator++()
+		{
+			cds_lfht_next(&_ht, &_it);
+			return *this;
+		}
+
+		bool operator==(const iterator& other) const noexcept
+		{
+			/* Compare pointed nodes by address. */
+			return other._it.node == _it.node;
+		}
+
+		bool operator!=(const iterator& other) const noexcept
+		{
+			return !(*this == other);
+		}
+
+		/* Dereference the iterator to access the contained element. */
+		ContainedType *operator*() const
+		{
+			auto *node = _it.node;
+
+			/* Throw an exception if dereferencing an end iterator. */
+			if (!node) {
+				LTTNG_THROW_OUT_OF_RANGE(
+					"Dereferenced an iterator at the end of a liburcu hashtable");
+			}
+
+			/* Retrieve the element from the node. */
+			return details::get_element_from_node<ContainedType, NodeType, Member>(
+				*node);
+		}
+
+	private:
+		iterator(cds_lfht& ht, const cds_lfht_iter& it) : _ht(ht), _it(it)
+		{
+		}
+
+		/* Reference to the hash table being iterated over. */
+		cds_lfht& _ht;
+		/* Native lfht iterator pointing to the current position. */
+		cds_lfht_iter _it;
+	};
+
+	explicit lfht_iteration_adapter(cds_lfht& ht) : _ht(ht)
+	{
+	}
+
+	/* Return an iterator to the beginning of the hash table. */
+	iterator begin() const noexcept
+	{
+		cds_lfht_iter it;
+
+		cds_lfht_first(&_ht, &it);
+		return iterator(_ht, it);
+	}
+
+	/* Return an iterator to the end of the hash table. */
+	iterator end() const noexcept
+	{
+		const cds_lfht_iter it = {};
+
+		return iterator(_ht, it);
+	}
+
+private:
+	/* Reference to the hash table being iterated over. */
+	cds_lfht& _ht;
+	/* RCU read lock held during the iteration. */
+	const lttng::urcu::read_lock_guard read_lock;
+};
+
 } /* namespace urcu */
 } /* namespace lttng */