Re: [ovs-dev] [PATCH v2 07/16] hmap: implement UB-safe hmap pop iterator

[0-day Robot]

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()
#76 FILE: tests/test-hmap.c:320:
assert(e == NULL);

Lines checked: 83, Warnings: 0, Errors: 1


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

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v2 07/16] hmap: implement UB-safe hmap pop iterator

[Adrian Moreno]

HMAP_FOR_EACH_POP iterator has an additional difficulty, which is the
use of two iterator variables of different types.

In order to re-write this loop in a UB-safe manner, create a iterator
struct to be used as loop variable.

Signed-off-by: Adrian Moreno 
---
 include/openvswitch/hmap.h | 31 +++
 tests/test-hmap.c  |  1 +
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/include/openvswitch/hmap.h b/include/openvswitch/hmap.h
index 356ea3582..283407e3a 100644
--- a/include/openvswitch/hmap.h
+++ b/include/openvswitch/hmap.h
@@ -200,26 +200,33 @@ bool hmap_contains(const struct hmap *, const struct 
hmap_node *);
  UPDATE_MULTIVAR(NODE,\
  ITER_VAR(NODE) = hmap_next(HMAP, ITER_VAR(NODE   \
 
-static inline struct hmap_node *
-hmap_pop_helper__(struct hmap *hmap, size_t *bucket) {
+struct hmap_pop_helper_iter__ {
+size_t bucket;
+struct hmap_node *node;
+};
+
+static inline void
+hmap_pop_helper__(struct hmap *hmap, struct hmap_pop_helper_iter__ *iter) {
 
-for (; *bucket <= hmap->mask; (*bucket)++) {
-struct hmap_node *node = hmap->buckets[*bucket];
+for (; iter->bucket <= hmap->mask; (iter->bucket)++) {
+struct hmap_node *node = hmap->buckets[iter->bucket];
 
 if (node) {
 hmap_remove(hmap, node);
-return node;
+iter->node = node;
+return;
 }
 }
-
-return NULL;
+iter->node = NULL;
 }
 
-#define HMAP_FOR_EACH_POP(NODE, MEMBER, HMAP)   \
-for (size_t bucket__ = 0;   \
- INIT_CONTAINER(NODE, hmap_pop_helper__(HMAP, __), MEMBER),  \
- (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER))\
- || ((NODE = NULL), false);)
+#define HMAP_FOR_EACH_POP(NODE, MEMBER, HMAP) \
+for (struct hmap_pop_helper_iter__ ITER_VAR(NODE) = { 0, NULL };  \
+ hmap_pop_helper__(HMAP, _VAR(NODE)),\
+ (ITER_VAR(NODE).node != NULL) ?  \
+(((NODE) = OBJECT_CONTAINING(ITER_VAR(NODE).node, \
+ NODE, MEMBER)),1):   \
+(((NODE) = NULL), 0);)
 
 static inline struct hmap_node *hmap_first(const struct hmap *);
 static inline struct hmap_node *hmap_next(const struct hmap *,
diff --git a/tests/test-hmap.c b/tests/test-hmap.c
index a40ac8953..47b475538 100644
--- a/tests/test-hmap.c
+++ b/tests/test-hmap.c
@@ -317,6 +317,7 @@ test_hmap_for_each_pop(hash_func *hash)
 i++;
 }
 assert(i == n);
+assert(e == NULL);
 
 hmap_destroy();
 }
-- 
2.34.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev