[ovs-dev,shadow,2/8] cmap: Allow CMAP_FOR_EACH to be nested without shadowing variables.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 lib/cmap.h | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

On Wed, Feb 28, 2018 at 12:00:57AM -0800, Justin Pettit wrote:
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---
>  lib/cmap.h | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/cmap.h b/lib/cmap.h
> index 5a72b65bec5e..2427a6293b64 100644
> --- a/lib/cmap.h
> +++ b/lib/cmap.h
> @@ -233,11 +233,20 @@ struct cmap_cursor {
>  struct cmap_cursor cmap_cursor_start(const struct cmap *);
>  void cmap_cursor_advance(struct cmap_cursor *);
>  
> -#define CMAP_FOR_EACH(NODE, MEMBER, CMAP)                       \
> -    for (struct cmap_cursor cursor__ = cmap_cursor_start(CMAP); \
> -         CMAP_CURSOR_FOR_EACH__(NODE, &cursor__, MEMBER);       \
> +/* Generate a unique name for the cursor with the __COUNTER__ macro to
> + * allow nesting of CMAP_FOR_EACH loops. */
> +#define CURSOR_JOIN2(x,y) x##y
> +#define CURSOR_JOIN(x, y) CURSOR_JOIN2(x,y)
> +#define CURSOR_NAME CURSOR_JOIN(cursor_, __COUNTER__)
> +
> +#define CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)           \
> +    for (struct cmap_cursor CURSOR_NAME = cmap_cursor_start(CMAP); \
> +         CMAP_CURSOR_FOR_EACH__(NODE, &CURSOR_NAME, MEMBER);       \
>          )
>  
> +#define CMAP_FOR_EACH(NODE, MEMBER, CMAP) \
> +          CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)
> +
>  static inline struct cmap_node *cmap_first(const struct cmap *);

Good work.

It's a little confusing to see CURSOR_NAME as both a macro name and a
macro argument name.  Could you get rid of the macro entirely, e.g. like
this?  Otherwise, two different names would be nice.

diff --git a/lib/cmap.h b/lib/cmap.h
index 4746c0b3c6f5..2662681c4e38 100644
--- a/lib/cmap.h
+++ b/lib/cmap.h
@@ -236,7 +236,6 @@ void cmap_cursor_advance(struct cmap_cursor *);
  * allow nesting of CMAP_FOR_EACH loops. */
 #define CURSOR_JOIN2(x,y) x##y
 #define CURSOR_JOIN(x, y) CURSOR_JOIN2(x,y)
-#define CURSOR_NAME CURSOR_JOIN(cursor_, __COUNTER__)
 
 #define CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)           \
     for (struct cmap_cursor CURSOR_NAME = cmap_cursor_start(CMAP); \
@@ -244,7 +243,8 @@ void cmap_cursor_advance(struct cmap_cursor *);
         )
 
 #define CMAP_FOR_EACH(NODE, MEMBER, CMAP) \
-          CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)
+          CMAP_FOR_EACH__(NODE, MEMBER, CMAP, \
+                          CURSOR_JOIN(cursor_, __COUNTER__))
 
 static inline struct cmap_node *cmap_first(const struct cmap *);
 

Acked-by: Ben Pfaff <blp@ovn.org>

> On Feb 28, 2018, at 1:18 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> Good work.
> 
> It's a little confusing to see CURSOR_NAME as both a macro name and a
> macro argument name.  Could you get rid of the macro entirely, e.g. like
> this?  Otherwise, two different names would be nice.
> 
> diff --git a/lib/cmap.h b/lib/cmap.h
> index 4746c0b3c6f5..2662681c4e38 100644
> --- a/lib/cmap.h
> +++ b/lib/cmap.h
> @@ -236,7 +236,6 @@ void cmap_cursor_advance(struct cmap_cursor *);
>  * allow nesting of CMAP_FOR_EACH loops. */
> #define CURSOR_JOIN2(x,y) x##y
> #define CURSOR_JOIN(x, y) CURSOR_JOIN2(x,y)
> -#define CURSOR_NAME CURSOR_JOIN(cursor_, __COUNTER__)
> 
> #define CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)           \
>     for (struct cmap_cursor CURSOR_NAME = cmap_cursor_start(CMAP); \
> @@ -244,7 +243,8 @@ void cmap_cursor_advance(struct cmap_cursor *);
>         )
> 
> #define CMAP_FOR_EACH(NODE, MEMBER, CMAP) \
> -          CMAP_FOR_EACH__(NODE, MEMBER, CMAP, CURSOR_NAME)
> +          CMAP_FOR_EACH__(NODE, MEMBER, CMAP, \
> +                          CURSOR_JOIN(cursor_, __COUNTER__))
> 
> static inline struct cmap_node *cmap_first(const struct cmap *);
> 
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

That is better.  Thanks!

--Justin