On 3/3/22 10:39, Dumitru Ceara wrote:
On 3/3/22 07:43, Adrian Moreno wrote:
Thanks for the review Dumitru,
A comment below.
On 2/25/22 13:06, Dumitru Ceara wrote:
On 2/16/22 15:27, Adrian Moreno wrote:
Multi-variable loop iterators avoid potential undefined behavior by
using an internal iterator variable to perform the iteration and only
referencing the containing object (via OBJECT_CONTAINING) if the
iterator has been validated via the second expression of the for
statement.
That way, the user can easily implement a loop that never tries to
obtain the object containing NULL or stack-allocated non-contained
nodes.
When the loop ends normally (not via "break;") the user-provided
variable is set to NULL.
Signed-off-by: Adrian Moreno <amore...@redhat.com>
---
Hi Adrian,
It feels a bit weird that these new macros are not used anywhere yet
(until we apply the next patches in the series). On the other hand I
don't think it's easy to split the series otherwise so, with a small nit
below:
Acked-by: Dumitru Ceara <dce...@redhat.com>
Thanks,
Dumitru
include/openvswitch/util.h | 44 ++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/include/openvswitch/util.h b/include/openvswitch/util.h
index 228b185c3..9c09e8aea 100644
--- a/include/openvswitch/util.h
+++ b/include/openvswitch/util.h
@@ -145,6 +145,50 @@ OVS_NO_RETURN void ovs_assert_failure(const char
*, const char *, const char *);
#define INIT_CONTAINER(OBJECT, POINTER, MEMBER) \
((OBJECT) = NULL, ASSIGN_CONTAINER(OBJECT, POINTER, MEMBER))
+
Nit: We don't need this extra line.
+/* Multi-variable container iterators.
+ *
+ * The following macros facilitate safe iteration over data structures
+ * contained in objects. It does so by using an internal iterator
variable of
+ * the type of the member object pointer (i.e: pointer to the data
structure).
+ */
+
+/* Multi-variable iterator variable name.
+ * Returns the name of the internal iterator variable.
+ */
+#define ITER_VAR(NAME) NAME ## __iterator__
+
+/* Multi-variable initialization. Creates an internal iterator
variable that
+ * points to the provided pointer. The type of the iterator variable is
+ * ITER_TYPE*. It must be the same type as &VAR->MEMBER.
+ *
+ * The _EXP version evaluates the extra expressions once.
+ */
+#define INIT_MULTIVAR(VAR, MEMBER, POINTER,
ITER_TYPE) \
+ INIT_MULTIVAR_EXP(VAR, MEMBER, POINTER, ITER_TYPE, (void) 0)
+
+#define INIT_MULTIVAR_EXP(VAR, MEMBER, POINTER, ITER_TYPE,
...) \
+ ITER_TYPE *ITER_VAR(VAR) = ( __VA_ARGS__ , (ITER_TYPE *) POINTER)
+
+/* Multi-variable condition.
+ * Evaluates the condition expression (that must be based on the
internal
+ * iterator variable). Only if the result of expression is true, the
OBJECT is
+ * set to the object containing the current value of the iterator
variable.
+ *
+ * It is up to the caller to make sure it is safe to run
OBJECT_CONTAINING on
+ * the pointers that verify the condition.
+ */
+#define CONDITION_MULTIVAR(VAR, MEMBER,
EXPR) \
+ ((EXPR)
? \
+ (((VAR) = OBJECT_CONTAINING(ITER_VAR(VAR), VAR, MEMBER)), 1)
: \
+ (((VAR) = NULL), 0))
+
+/* Multi-variable update.
+ * Evaluates the expresssion that is supposed to update the iterator
variable.
+ */
+#define UPDATE_MULTIVAR(VAR,
EXPR) \
+ ((EXPR), (VAR) = NULL)
+
Setting VAR = NULL here may not really be needed. The CONDITION is
evaluated just after the UPDATE stage and it sets VAR appropriately.
Good point. The same is true for the _SAFE_* versions in the next patch
too, right?
Yes.
If you agree, I'll remove the entire UPDATE macro alongside the extra
line and the extra "s" in "expression" in the next version.
Sounds good to me, thanks!
Ack, thanks.
--
Adrián Moreno
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
