diff mbox series

[ovs-dev,10/16] hindex: use multi-variable iterators

Message ID 20220214094003.3844268-11-amorenoz@redhat.com
State Superseded
Headers show
Series Fix undefined behavior in loop macros | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Adrian Moreno Feb. 14, 2022, 9:39 a.m. UTC 
Re-write hindex's loops using multi-variable helpers.

For safe loops, use the LONG version to maintain backwards
compatibility.

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 lib/hindex.h        | 42 +++++++++++++++++++++++-------------------
 tests/test-hindex.c |  6 ++++++
 2 files changed, 29 insertions(+), 19 deletions(-)

Comments

0-day Robot Feb. 14, 2022, 11:16 a.m. UTC | #1 
Bleep bloop.  Greetings Adrian Moreno, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Use ovs_assert() in place of assert()
#92 FILE: tests/test-hindex.c:269:
                    assert(next == NULL);

ERROR: Use ovs_assert() in place of assert()
#94 FILE: tests/test-hindex.c:271:
                    assert(&next->node == hindex_next(&hindex, &e->node));

ERROR: Use ovs_assert() in place of assert()
#103 FILE: tests/test-hindex.c:289:
            assert(next == NULL);

Lines checked: 110, Warnings: 0, Errors: 3


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/lib/hindex.h b/lib/hindex.h
index 876c5a9e3..c20752c25 100644
--- a/lib/hindex.h
+++ b/lib/hindex.h
@@ -128,18 +128,20 @@  void hindex_remove(struct hindex *, struct hindex_node *);
  * Evaluates HASH only once.
  */
 #define HINDEX_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, HINDEX)               \
-    for (INIT_CONTAINER(NODE, hindex_node_with_hash(HINDEX, HASH), MEMBER); \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                     \
-         ASSIGN_CONTAINER(NODE, (NODE)->MEMBER.s, MEMBER))
+    for (INIT_MULTIVAR(NODE, MEMBER, hindex_node_with_hash(HINDEX, HASH));  \
+         CONDITION_MULTIVAR(NODE, MEMBER, ITER_VAR(NODE) != NULL);          \
+         UPDATE_MULTIVAR(NODE, ITER_VAR(NODE) = ITER_VAR(NODE)->s))
 
 /* Safe when NODE may be freed (not needed when NODE may be removed from the
  * hash map but its members remain accessible and intact). */
-#define HINDEX_FOR_EACH_WITH_HASH_SAFE(NODE, NEXT, MEMBER, HASH, HINDEX) \
-    for (INIT_CONTAINER(NODE, hindex_node_with_hash(HINDEX, HASH), MEMBER); \
-         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)                 \
-          ? INIT_CONTAINER(NEXT, (NODE)->MEMBER.s, MEMBER), 1           \
-          : 0);                                                         \
-         (NODE) = (NEXT))
+#define HINDEX_FOR_EACH_WITH_HASH_SAFE(NODE, NEXT, MEMBER, HASH, HINDEX)    \
+    for (INIT_MULTIVAR_SAFE_LONG(NODE, NEXT, MEMBER,                        \
+                                hindex_node_with_hash(HINDEX, HASH));       \
+         CONDITION_MULTIVAR_SAFE_LONG(NODE, NEXT, MEMBER,                   \
+                                      ITER_VAR(NODE) != NULL,               \
+                                      ITER_VAR(NEXT) = ITER_VAR(NODE)->s,   \
+                                      ITER_VAR(NEXT) != NULL);              \
+         UPDATE_MULTIVAR_SAFE_LONG(NODE, NEXT))
 
 /* Returns the head node in 'hindex' with the given 'hash', or a null pointer
  * if no nodes have that hash value. */
@@ -157,19 +159,21 @@  hindex_node_with_hash(const struct hindex *hindex, size_t hash)
 /* Iteration. */
 
 /* Iterates through every node in HINDEX. */
-#define HINDEX_FOR_EACH(NODE, MEMBER, HINDEX)                           \
-    for (INIT_CONTAINER(NODE, hindex_first(HINDEX), MEMBER);            \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                 \
-         ASSIGN_CONTAINER(NODE, hindex_next(HINDEX, &(NODE)->MEMBER), MEMBER))
+#define HINDEX_FOR_EACH(NODE, MEMBER, HINDEX)                                 \
+    for (INIT_MULTIVAR(NODE, MEMBER, hindex_first(HINDEX));                   \
+         CONDITION_MULTIVAR(NODE, MEMBER, ITER_VAR(NODE) != NULL);            \
+         UPDATE_MULTIVAR(NODE,                                                \
+                         ITER_VAR(NODE) = hindex_next(HINDEX, ITER_VAR(NODE))))
 
 /* Safe when NODE may be freed (not needed when NODE may be removed from the
  * hash index but its members remain accessible and intact). */
-#define HINDEX_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HINDEX)              \
-    for (INIT_CONTAINER(NODE, hindex_first(HINDEX), MEMBER);          \
-         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)                 \
-          ? INIT_CONTAINER(NEXT, hindex_next(HINDEX, &(NODE)->MEMBER), MEMBER), 1 \
-          : 0);                                                         \
-         (NODE) = (NEXT))
+#define HINDEX_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HINDEX)                      \
+    for (INIT_MULTIVAR_SAFE_LONG(NODE, NEXT, MEMBER, hindex_first(HINDEX));   \
+         CONDITION_MULTIVAR_SAFE_LONG(NODE, NEXT, MEMBER,                     \
+                                      ITER_VAR(NODE) != NULL,                 \
+                        ITER_VAR(NEXT) = hindex_next(HINDEX, ITER_VAR(NODE)), \
+                                      ITER_VAR(NEXT) != NULL);                \
+         UPDATE_MULTIVAR_SAFE_LONG(NODE, NEXT))
 
 struct hindex_node *hindex_first(const struct hindex *);
 struct hindex_node *hindex_next(const struct hindex *,
diff --git a/tests/test-hindex.c b/tests/test-hindex.c
index af06be5fc..95e49284e 100644
--- a/tests/test-hindex.c
+++ b/tests/test-hindex.c
@@ -265,6 +265,11 @@  test_hindex_for_each_safe(hash_func *hash)
             i = 0;
             n_remaining = n;
             HINDEX_FOR_EACH_SAFE (e, next, node, &hindex) {
+                if (hindex_next(&hindex, &e->node) == NULL) {
+                    assert(next == NULL);
+                } else {
+                    assert(&next->node == hindex_next(&hindex, &e->node));
+                }
                 assert(i < n);
                 if (pattern & (1ul << e->value)) {
                     size_t j;
@@ -281,6 +286,7 @@  test_hindex_for_each_safe(hash_func *hash)
                 i++;
             }
             assert(i == n);
+            assert(next == NULL);
 
             for (i = 0; i < n; i++) {
                 if (pattern & (1ul << i)) {