hmap: Ensure iterator is NULL after iteration.
authorRussell Bryant <rbryant@redhat.com>
Tue, 18 Aug 2015 18:43:18 +0000 (11:43 -0700)
committerBen Pfaff <blp@nicira.com>
Fri, 21 Aug 2015 21:24:08 +0000 (14:24 -0700)
The HMAP_FOR_EACH_()* macros had a usability issue where the iterator
was only NULL at the completion of iteration if the hmap_node was the
first struct member.  This change ensures that the iterator is set to
NULL when iteration ends normally without a 'break'.

Signed-off-by: Russell Bryant <rbryant@redhat.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>
lib/hmap.h

index 345bf7f..cab567e 100644 (file)
@@ -124,24 +124,18 @@ struct hmap_node *hmap_random_node(const struct hmap *);
  *
  * HASH is only evaluated once.
  *
- *
- * Warning
- * -------
- *
- * When the loop terminates, &NODE->MEMBER will equal NULL.  Unless MEMBER is
- * the first member in its struct, this means that NODE itself will not be
- * NULL.
- *
- * (This is true for all of the HMAP_FOR_EACH_*() macros.)
+ * When the loop terminates normally, meaning the iteration has completed
+ * without using 'break', NODE will be NULL.  This is true for all of the
+ * HMAP_FOR_EACH_*() macros.
  */
 #define HMAP_FOR_EACH_WITH_HASH(NODE, MEMBER, HASH, HMAP)               \
     for (INIT_CONTAINER(NODE, hmap_first_with_hash(HMAP, HASH), MEMBER); \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                  \
+         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
          ASSIGN_CONTAINER(NODE, hmap_next_with_hash(&(NODE)->MEMBER),   \
                           MEMBER))
 #define HMAP_FOR_EACH_IN_BUCKET(NODE, MEMBER, HASH, HMAP)               \
     for (INIT_CONTAINER(NODE, hmap_first_in_bucket(HMAP, HASH), MEMBER); \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                  \
+         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
          ASSIGN_CONTAINER(NODE, hmap_next_in_bucket(&(NODE)->MEMBER), MEMBER))
 
 static inline struct hmap_node *hmap_first_with_hash(const struct hmap *,
@@ -158,14 +152,14 @@ bool hmap_contains(const struct hmap *, const struct hmap_node *);
 /* Iterates through every node in HMAP. */
 #define HMAP_FOR_EACH(NODE, MEMBER, HMAP)                               \
     for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER);                \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                  \
+         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
          ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER))
 
 /* 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 HMAP_FOR_EACH_SAFE(NODE, NEXT, MEMBER, HMAP)                    \
     for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER);                \
-         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)                  \
+         ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL) \
           ? INIT_CONTAINER(NEXT, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER), 1 \
           : 0);                                                         \
          (NODE) = (NEXT))
@@ -173,7 +167,7 @@ bool hmap_contains(const struct hmap *, const struct hmap_node *);
 /* Continues an iteration from just after NODE. */
 #define HMAP_FOR_EACH_CONTINUE(NODE, MEMBER, HMAP)                      \
     for (ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER); \
-         NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER);                  \
+         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = NULL); \
          ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), MEMBER))
 
 static inline struct hmap_node *hmap_first(const struct hmap *);