diff mbox series

[ovs-dev,1/1] odp-util: Fix Sparse warning.

Message ID 1515693538-28789-1-git-send-email-ian.stokes@intel.com
State Superseded
Headers show
Series [ovs-dev,1/1] odp-util: Fix Sparse warning. | expand

Commit Message

Stokes, Ian Jan. 11, 2018, 5:58 p.m. UTC
Sparse complains with warning: incorrect type in argument 1 (different
base types) in function parse_odp_userspace_action due to a call to
htonll(rule_cookie). Rule_cookie variable is already ovs_be64 so fix
this by removing the call to htonll as there is no need to convert for
use with put_32aligned_be64().

CC: Justin Pettit <jpettit@ovn.org>
Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.")
Signed-off-by: Ian Stokes <ian.stokes@intel.com>
---
 lib/odp-util.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Jan. 11, 2018, 6:27 p.m. UTC | #1
On Thu, Jan 11, 2018 at 05:58:58PM +0000, Ian Stokes wrote:
> Sparse complains with warning: incorrect type in argument 1 (different
> base types) in function parse_odp_userspace_action due to a call to
> htonll(rule_cookie). Rule_cookie variable is already ovs_be64 so fix
> this by removing the call to htonll as there is no need to convert for
> use with put_32aligned_be64().
> 
> CC: Justin Pettit <jpettit@ovn.org>
> Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.")
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>

Thanks for the fix.

I noticed that one of the tests broke when I applied this, so I found
the problem and sent v2:
        https://patchwork.ozlabs.org/patch/859232/

Justin?
Justin Pettit Jan. 11, 2018, 6:57 p.m. UTC | #2
> On Jan 11, 2018, at 9:58 AM, Ian Stokes <ian.stokes@intel.com> wrote:
> 
> Sparse complains with warning: incorrect type in argument 1 (different
> base types) in function parse_odp_userspace_action due to a call to
> htonll(rule_cookie). Rule_cookie variable is already ovs_be64 so fix
> this by removing the call to htonll as there is no need to convert for
> use with put_32aligned_be64().
> 
> CC: Justin Pettit <jpettit@ovn.org>
> Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller actions.")
> Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> ---
> lib/odp-util.c |    3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index f8c84e1..fa1a5c9 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -1245,8 +1245,7 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
>             cookie.controller.continuation = continuation ? true : false;
>             cookie.controller.reason = reason;
>             cookie.controller.recirc_id = recirc_id;
> -            put_32aligned_be64(&cookie.controller.rule_cookie,
> -                               htonll(rule_cookie));
> +            put_32aligned_be64(&cookie.controller.rule_cookie, rule_cookie);
>             cookie.controller.controller_id = controller_id;
>             cookie.controller.max_len = max_len;
>        } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) {

Thanks for pointing out the issue, Ian.  I don't think this is the solution, though, since it causes unit test failure.  I'll send a follow-up patch that should address the issue.

--Justin
Stokes, Ian Jan. 11, 2018, 8:12 p.m. UTC | #3
> -----Original Message-----
> From: Justin Pettit [mailto:jpettit@ovn.org]
> Sent: Thursday, January 11, 2018 6:58 PM
> To: Stokes, Ian <ian.stokes@intel.com>
> Cc: dev@openvswitch.org
> Subject: Re: [PATCH 1/1] odp-util: Fix Sparse warning.
> 
> 
> 
> > On Jan 11, 2018, at 9:58 AM, Ian Stokes <ian.stokes@intel.com> wrote:
> >
> > Sparse complains with warning: incorrect type in argument 1 (different
> > base types) in function parse_odp_userspace_action due to a call to
> > htonll(rule_cookie). Rule_cookie variable is already ovs_be64 so fix
> > this by removing the call to htonll as there is no need to convert for
> > use with put_32aligned_be64().
> >
> > CC: Justin Pettit <jpettit@ovn.org>
> > Fixes: d39ec23de384 ("ofproto-dpif: Don't slow-path controller
> > actions.")
> > Signed-off-by: Ian Stokes <ian.stokes@intel.com>
> > ---
> > lib/odp-util.c |    3 +--
> > 1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c index f8c84e1..fa1a5c9
> > 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -1245,8 +1245,7 @@ parse_odp_userspace_action(const char *s, struct
> ofpbuf *actions)
> >             cookie.controller.continuation = continuation ? true :
> false;
> >             cookie.controller.reason = reason;
> >             cookie.controller.recirc_id = recirc_id;
> > -            put_32aligned_be64(&cookie.controller.rule_cookie,
> > -                               htonll(rule_cookie));
> > +            put_32aligned_be64(&cookie.controller.rule_cookie,
> > + rule_cookie);
> >             cookie.controller.controller_id = controller_id;
> >             cookie.controller.max_len = max_len;
> >        } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) {
> 
> Thanks for pointing out the issue, Ian.  I don't think this is the
> solution, though, since it causes unit test failure.  I'll send a follow-
> up patch that should address the issue.

Ah, I kicked travis off before leaving the office and after seeing the initial gcc compilation test was now passing, I thought it was good, lesson learned :(.

Ian
> 
> --Justin
>
diff mbox series

Patch

diff --git a/lib/odp-util.c b/lib/odp-util.c
index f8c84e1..fa1a5c9 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1245,8 +1245,7 @@  parse_odp_userspace_action(const char *s, struct ofpbuf *actions)
             cookie.controller.continuation = continuation ? true : false;
             cookie.controller.reason = reason;
             cookie.controller.recirc_id = recirc_id;
-            put_32aligned_be64(&cookie.controller.rule_cookie,
-                               htonll(rule_cookie));
+            put_32aligned_be64(&cookie.controller.rule_cookie, rule_cookie);
             cookie.controller.controller_id = controller_id;
             cookie.controller.max_len = max_len;
        } else if (ovs_scan(&s[n], ",userdata(%n", &n1)) {