[ovs-dev] hmap.h: Fix Coverity false positive

Message ID	1584654437-124095-1-git-send-email-ua1422@gmail.com
State	Accepted
Headers	show Return-Path: <ovs-dev-bounces@openvswitch.org> From: ua1422@gmail.com To: dev@openvswitch.org Date: Thu, 19 Mar 2020 14:47:17 -0700 Message-Id: <1584654437-124095-1-git-send-email-ua1422@gmail.com> Cc: Usman Ansari <u1422@gmail.com> Subject: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" <ovs-dev-bounces@openvswitch.org>
Series	[ovs-dev] hmap.h: Fix Coverity false positive \| expand [ovs-dev] hmap.h: Fix Coverity false positive

Message ID

1584654437-124095-1-git-send-email-ua1422@gmail.com

State

Accepted

Headers

From: ua1422@gmail.com
To: dev@openvswitch.org
Date: Thu, 19 Mar 2020 14:47:17 -0700
Message-Id: <1584654437-124095-1-git-send-email-ua1422@gmail.com>
Cc: Usman Ansari <u1422@gmail.com>
Subject: [ovs-dev] [PATCH] hmap.h: Fix Coverity false positive
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: ovs-dev-bounces@openvswitch.org
Sender: "dev" <ovs-dev-bounces@openvswitch.org>

Series

[ovs-dev] hmap.h: Fix Coverity false positive | expand

Commit Message

Usman S. Ansari March 19, 2020, 9:47 p.m. UTC

From: Usman Ansari <ua1422@gmail.com>

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)'.
"make check" passes for this change
Coverity reports over 500 errors resolved

Suggested-by: Ben Pfaff <blp@ovn.org>
Signed-off-by: Usman Ansari <u1422@gmail.com>
---
 include/openvswitch/hmap.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Ben Pfaff March 19, 2020, 10:09 p.m. UTC | #1

On Thu, Mar 19, 2020 at 02:47:17PM -0700, ua1422@gmail.com wrote:
> From: Usman Ansari <ua1422@gmail.com>
> 
> 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)'.
> "make check" passes for this change
> Coverity reports over 500 errors resolved
> 
> Suggested-by: Ben Pfaff <blp@ovn.org>
> Signed-off-by: Usman Ansari <u1422@gmail.com>

Thanks.  I folded the too-long lines, fixed the typo in your
Signed-off-by line, and applied this to master.

0-day Robot March 19, 2020, 10:10 p.m. UTC | #2

Bleep bloop.  Greetings Usman Ansari, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Usman Ansari <ua1422@gmail.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Usman Ansari <u1422@gmail.com>
WARNING: Line is 85 characters long (recommended limit is 79)
#28 FILE: include/openvswitch/hmap.h:139:
         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \

WARNING: Line is 85 characters long (recommended limit is 79)
#34 FILE: include/openvswitch/hmap.h:144:
         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \

WARNING: Line is 85 characters long (recommended limit is 79)
#43 FILE: include/openvswitch/hmap.h:173:
         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \

WARNING: Line is 85 characters long (recommended limit is 79)
#52 FILE: include/openvswitch/hmap.h:182:
         ((NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false) \

WARNING: Line is 85 characters long (recommended limit is 79)
#61 FILE: include/openvswitch/hmap.h:193:
         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false); \

WARNING: Line is 84 characters long (recommended limit is 79)
#70 FILE: include/openvswitch/hmap.h:214:
         (NODE != OBJECT_CONTAINING(NULL, NODE, MEMBER)) || ((NODE = NULL), false);)

Lines checked: 76, Warnings: 7, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot

diff --git a/include/openvswitch/hmap.h b/include/openvswitch/hmap.h
index 8aea9c2..74a0929 100644
--- a/include/openvswitch/hmap.h
+++ b/include/openvswitch/hmap.h
@@ -136,12 +136,12 @@  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 +170,7 @@  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 +179,7 @@  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 +190,7 @@  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 +211,7 @@  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 *,

[ovs-dev] hmap.h: Fix Coverity false positive

Commit Message

Comments

Patch