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
