I am only sending patch for the macro, for other I will send additional patch(s)
On Wed, Mar 18, 2020 at 12:33 PM Usman Ansari <[email protected]> wrote: > Will resubmit the patch. > > Coverity says 80 bugs fixed from this change. > > On Wed, Mar 18, 2020 at 12:19 PM William Tu <[email protected]> wrote: > > > On Wed, Mar 18, 2020 at 11:55 AM Ben Pfaff <[email protected]> wrote: > > > > > > On Wed, Mar 18, 2020 at 11:43:29AM -0700, Usman Ansari wrote: > > > > From Ben Pfaff [email protected] > > > > > > > > > > > > Coverity reports incorrect expression for HMAP_FOR_EACH_WITH_HASH > macro > > > > This patch fixes this issue > > > > "make check" passes for this change > > > > Coverity reports 80 errors resolved > > > > > > > > Signed-off-by: Usman Ansari <[email protected]> > > > > > > Would you please rephrase the title of the patch? There is nothing > > > incorrect about the expression, it is just that Coverity complains > about > > > it. > > > > > > Thanks, > > > > > > Ben. > > > > Hi Usman, > > > > How about change the title to "hmap: Fix Coverity false positive."? > > > > Coverity reports a false positive below: > > Incorrect expression, Assign_where_compare_meant: use of "=" > > where "==" may have been intended. > > Fixed it by rewriting '(NODE = NULL)' as '((NODE = NULL), false)'. > > > > BTW, there are not only this place needed to fix. Please see diff below. > > With this fix, you should see it fixes around 500 coverity defects. > > > > --- a/include/openvswitch/hmap.h > > +++ b/include/openvswitch/hmap.h > > @@ -136,12 +136,14 @@ struct hmap_node *hmap_random_node(const struct > hmap > > *); > > */ > > #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 = > > NULL); \ > > + (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false); \ > > 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 = > > NULL); \ > > + (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false); \ > > ASSIGN_CONTAINER(NODE, hmap_next_in_bucket(&(NODE)->MEMBER), > > MEMBER)) > > > > static inline struct hmap_node *hmap_first_with_hash(const struct hmap > *, > > @@ -170,7 +172,8 @@ bool hmap_contains(const struct hmap *, const > > struct hmap_node *); > > HMAP_FOR_EACH_INIT(NODE, MEMBER, HMAP, (void) 0) > > #define HMAP_FOR_EACH_INIT(NODE, MEMBER, HMAP, ...) > \ > > for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER), __VA_ARGS__; > \ > > - (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = > > NULL); \ > > + (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false); \ > > ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), > MEMBER)) > > > > /* Safe when NODE may be freed (not needed when NODE may be removed from > > the > > @@ -179,7 +182,8 @@ bool hmap_contains(const struct hmap *, const > > struct hmap_node *); > > HMAP_FOR_EACH_SAFE_INIT(NODE, NEXT, MEMBER, HMAP, (void) 0) > > #define HMAP_FOR_EACH_SAFE_INIT(NODE, NEXT, MEMBER, HMAP, ...) > \ > > for (INIT_CONTAINER(NODE, hmap_first(HMAP), MEMBER), __VA_ARGS__; > \ > > - ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = > > NULL) \ > > + ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false) \ > > ? INIT_CONTAINER(NEXT, hmap_next(HMAP, &(NODE)->MEMBER), > > MEMBER), 1 \ > > : 0); > \ > > (NODE) = (NEXT)) > > @@ -190,7 +194,8 @@ bool hmap_contains(const struct hmap *, const > > struct hmap_node *); > > #define HMAP_FOR_EACH_CONTINUE_INIT(NODE, MEMBER, HMAP, ...) > \ > > for (ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), > > MEMBER), \ > > __VA_ARGS__; > \ > > - (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = > > NULL); \ > > + (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false); \ > > ASSIGN_CONTAINER(NODE, hmap_next(HMAP, &(NODE)->MEMBER), > MEMBER)) > > > > static inline struct hmap_node * > > @@ -211,7 +216,8 @@ hmap_pop_helper__(struct hmap *hmap, size_t *bucket) > { > > #define HMAP_FOR_EACH_POP(NODE, MEMBER, HMAP) > > \ > > for (size_t bucket__ = 0; > > \ > > INIT_CONTAINER(NODE, hmap_pop_helper__(HMAP, &bucket__), > > MEMBER), \ > > - (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || (NODE = > > NULL);) > > + (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || \ > > + ((NODE = NULL), false);) > > > > static inline struct hmap_node *hmap_first(const struct hmap *); > > static inline struct hmap_node *hmap_next(const struct hmap *, > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
