[ovs-dev] [PATCH v2 06/26] ofproto: Add generic ofproto_collection.

[Ben Pfaff]

On Thu, Jul 28, 2016 at 05:55:58PM -0700, Jarno Rajahalme wrote:
> Define rule_collection in terms of a new ofproto_collection.  This
> makes it easier to add other types of collections later.
> 
> This patch makes no functional changes.
> 
> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>

Is there anything ofproto-specific about these collections?  They might
be appropriate as a generic data structure in lib/.

At some point it becomes easier to implement this kind of thing as an
#include file than as a long \-delimited macro.  Then you also get the
benefit that you can define macros in the #include file.

"sparse" warns:
../ofproto/ofproto.c:4363:5: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:4528:5: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5091:9: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5111:9: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5176:9: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5203:9: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5255:5: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5277:9: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5349:5: warning: Using plain integer as NULL pointer
../ofproto/ofproto.c:5717:5: warning: Using plain integer as NULL pointer

which is because C rules say that both sides of the ?: here get
converted to a pointer type:

#define RULE_COLLECTION_FOR_EACH(RULE, RULES) \
    for (size_t i__ = 0; \
         i__ < rule_collection_n(RULES) \
             ? (RULE = rule_collection_rules(RULES)[i__]) : false;      \
         i__++)

I suggest:

diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index a82a398..9197813 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -540,14 +540,14 @@ DECL_COLLECTION(struct rule *, rule)
 #define RULE_COLLECTION_FOR_EACH(RULE, RULES) \
     for (size_t i__ = 0; \
          i__ < rule_collection_n(RULES) \
-             ? (RULE = rule_collection_rules(RULES)[i__]) : false;      \
+             ? (RULE = rule_collection_rules(RULES)[i__]) != NULL: false; \
          i__++)
 
 #define RULE_COLLECTIONS_FOR_EACH(RULE1, RULE2, RULES1, RULES2)        \
     for (size_t i__ = 0;                                                   \
          i__ < rule_collection_n(RULES1)                               \
              ? ((RULE1 = rule_collection_rules(RULES1)[i__]),          \
-                (RULE2 = rule_collection_rules(RULES2)[i__]))          \
+                (RULE2 = rule_collection_rules(RULES2)[i__]) != NULL)  \
              : false;                                                  \
          i__++)

RULE_COLLECTIONS_FOR_EACH could really use a comment describing what it
does.  I think that it pairwise traverses both collections, stopping
when it runs out of rules in either collection.

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