> On Jul 29, 2016, at 12:46 PM, Ben Pfaff <b...@ovn.org> wrote: > > 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 <ja...@ovn.org> > > Is there anything ofproto-specific about these collections? They might > be appropriate as a generic data structure in lib/. >
There is a small dependency for the _ref() and _unref() functions. I added lib/object-collection.[ch] for the generic parts, and kept the ofproto dependent parts in ofproto-provider.h. > 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. Elaborate a bit, I don't get what you mean. > > "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: > Thanks. > 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. > It actually has a requirement that both collections are of the same size. I added a comment. > Acked-by: Ben Pfaff <b...@ovn.org> Thanks for the review! Jarno _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
