On 2024-09-17 18:11, Konstantin Ananyev wrote:
+
+/**
+ * Get pointer to lcore variable instance with the specified lcore id.
+ *
+ * @param lcore_id
+ * The lcore id specifying which of the @c RTE_MAX_LCORE value
+ * instances should be accessed. The lcore id need not be valid
+ * (e.g., may be @ref LCORE_ID_ANY), but in such a case, the pointer
+ * is also not valid (and thus should not be dereferenced).
+ * @param handle
+ * The lcore variable handle.
+ */
+#define RTE_LCORE_VAR_LCORE_VALUE(lcore_id, handle) \
+ ((typeof(handle))rte_lcore_var_lcore_ptr(lcore_id, handle))
+
+/**
+ * Get pointer to lcore variable instance of the current thread.
+ *
+ * May only be used by EAL threads and registered non-EAL threads.
+ */
+#define RTE_LCORE_VAR_VALUE(handle) \
+ RTE_LCORE_VAR_LCORE_VALUE(rte_lcore_id(), handle)
Would it make sense to check that rte_lcore_id() != LCORE_ID_ANY?
After all if people do not want this extra check, they can probably use
RTE_LCORE_VAR_LCORE_VALUE(rte_lcore_id(), handle)
explicitly.
It would make sense, if it was an RTE_ASSERT(). Otherwise, I don't think
so. Attempting to gracefully handle API violations is bad practice, imo.
Ok, RTE_ASSERT() might be a good compromise.
As I said in another mail for that thread, I wouldn't insist here.
After a having a closer look at this issue, I'm not so sure any more.
Such an assertion would disallow the use of the macros to retrieve a
potentially-invalid pointer, which is then never used, in case it is
invalid.
+
+/**
+ * Iterate over each lcore id's value for an lcore variable.
+ *
+ * @param value
+ * A pointer successively set to point to lcore variable value
+ * corresponding to every lcore id (up to @c RTE_MAX_LCORE).
+ * @param handle
+ * The lcore variable handle.
+ */
+#define RTE_LCORE_VAR_FOREACH_VALUE(value, handle) \
+ for (unsigned int lcore_id = \
+ (((value) = RTE_LCORE_VAR_LCORE_VALUE(0, handle)), 0); \
+ lcore_id < RTE_MAX_LCORE; \
+ lcore_id++, (value) = RTE_LCORE_VAR_LCORE_VALUE(lcore_id, handle))
Might be a bit better (and safer) to make lcore_id a macro parameter?
I.E.:
define RTE_LCORE_VAR_FOREACH_VALUE(value, handle, lcore_id) \
for ((lcore_id) = ...
Why?
Variable with the same name (and type) can be defined by user before the loop,
With the intention to use it inside the loop.
Just like it happens here (in patch #2):
+ unsigned int lcore_id;
.....
+ /* take the opportunity to test the foreach macro */
+ int *v;
+ lcore_id = 0;
+ RTE_LCORE_VAR_FOREACH_VALUE(v, test_int) {
+ TEST_ASSERT_EQUAL(states[lcore_id].new_value, *v,
+ "Unexpected value on lcore %d during "
+ "iteration", lcore_id);
+ lcore_id++;
+ }
+
Indeed. I'll change it. I suppose you could also have issues if you
nested the macro, although those could be solved by using something like
__COUNTER__ to create a unique name.
Supplying the variable name does defeat part of the purpose of the
RTE_LCORE_VAR_FOREACH_VALUE.
