hmap: Add extra build-time iteration checks for types derived from hmap.

Some of our data structures derived from hmap use the same member names.
This means it's possible to confuse them in iteration, e.g. to iterate a
shash with SIMAP_FOR_EACH.  Of course this will crash at runtime, but it
seems even better to catch it at compile time.

An alternative would be to use unique member names, e.g. shash_map and
simap_map instead of just map.  I like short names, though.

It's kind of nasty that we need support from the hmap code to do this.
An alternative would be to insert the build assertions as statements before
the for loop.  But that would cause nasty surprises if someone forgets the
{} around a block of statements; even though the OVS coding style requires
them in all cases, I suspect that programmers doing debugging, etc. tend
to omit them sometimes.

It's not actually necessary to have multiple variants of these macros,
e.g. one can write a C99-compliant HMAP_FOR_EACH that accepts 3 or 4 or
more arguments.  But such a macro is harder to read, so I don't know
whether this is a good tradeoff.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Andy Zhou <azhou@ovn.org>

1 files changed, 12 insertions, 6 deletions

diff --git a/lib/smap.h b/lib/smap.h
index 489497a0f..7562f3841 100644
--- a/lib/smap.h
+++ b/lib/smap.h
@@ -1,4 +1,4 @@
-/* Copyright (c) 2012, 2014, 2015 Nicira, Inc.
+/* Copyright (c) 2012, 2014, 2015, 2016 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -33,11 +33,17 @@ struct smap_node {
 
 #define SMAP_INITIALIZER(SMAP) { HMAP_INITIALIZER(&(SMAP)->map) }
 
-#define SMAP_FOR_EACH(SMAP_NODE, SMAP) \
-    HMAP_FOR_EACH (SMAP_NODE, node, &(SMAP)->map)
-
-#define SMAP_FOR_EACH_SAFE(SMAP_NODE, NEXT, SMAP) \
-    HMAP_FOR_EACH_SAFE (SMAP_NODE, NEXT, node, &(SMAP)->map)
+#define SMAP_FOR_EACH(SMAP_NODE, SMAP)                                  \
+    HMAP_FOR_EACH_INIT (SMAP_NODE, node, &(SMAP)->map,                  \
+                        BUILD_ASSERT_TYPE(SMAP_NODE, struct smap_node *), \
+                        BUILD_ASSERT_TYPE(SMAP, struct smap *))
+
+#define SMAP_FOR_EACH_SAFE(SMAP_NODE, NEXT, SMAP)           \
+    HMAP_FOR_EACH_SAFE_INIT (                               \
+        SMAP_NODE, NEXT, node, &(SMAP)->map,                \
+        BUILD_ASSERT_TYPE(SMAP_NODE, struct smap_node *),   \
+        BUILD_ASSERT_TYPE(NEXT, struct smap_node *),        \
+        BUILD_ASSERT_TYPE(SMAP, struct smap *))
 
 /* Initializer for an immutable struct smap 'SMAP' that contains a single
  * 'KEY'-'VALUE' pair, e.g.