All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: David Marchand <david.marchand@redhat.com>
Cc: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	dev@dpdk.org, "Richardson, Bruce" <bruce.richardson@intel.com>,
	Ori Kam <orika@mellanox.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>,
	Andrew Rybchenko <arybchenko@solarflare.com>,
	"Zhang, Xiao" <xiao.zhang@intel.com>
Subject: Re: [dpdk-dev] [PATCH 3/3] ethdev: fix build warning on 64-bit value
Date: Tue, 28 Apr 2020 11:27:44 +0200	[thread overview]
Message-ID: <1847700.fxN4lLDhpz@thomas> (raw)
In-Reply-To: <CAJFAV8wkVZ5TiYaXR5QPV89kZ6t4+xOcMLkDEkENgD3eSrCxWA@mail.gmail.com>

27/04/2020 16:00, David Marchand:
> On Mon, Apr 27, 2020 at 3:47 PM Ananyev, Konstantin wrote:
> > From: dev <dev-bounces@dpdk.org> On Behalf Of David Marchand
> > > On Mon, Apr 27, 2020 at 3:34 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > On Mon, Apr 27, 2020 at 03:23:41PM +0200, David Marchand wrote:
> > > > > Building OVS with dpdk, sparse complains about 64-bit constant being
> > > > > passed as a normal integer that can't fit it:
> > > > > error: constant 0xffffffffffffffff is so big it is unsigned long
> > > > >
> > > > > Fixes: ecbc8570131d ("ethdev: add PFCP header to flow API")
> > > > >
> > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > > ---
> > > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > >  static const struct rte_flow_item_pfcp rte_flow_item_pfcp_mask = {
> > > > >       .s_field = 0x01,
> > > > > -     .seid = RTE_BE64(0xffffffffffffffff),
> > > > > +     .seid = RTE_BE64(UINT64_C(0xffffffffffffffff)),
> > > >
> > > > Rather than cast, why not put "ULL" at the end. If we are going to cast,
> > > > why not just put "-1" in to save some digits.
> > >
> > > I preferred this form in the hope future developers who want
> > > 0x0fffffffffffffff will copy/paste this.
> > >
> >
> > As I remember there should be UINT64_MAX in stdint.h.
> 
> Yes, we could go with:
> +     .seid = RTE_BE64(UINT64_MAX),
> 
> And then next time, for any value like 0x0fff ffff ffff ffff (had to
> group the digits of what I had written), pretty sure we will miss this
> and I will catch it only when building ovs.

I agree with David (in general and especially here).

RTE_BE64 is required for static analyzers and is an explicit info.

UINT64_C is better than ULL suffix because it
	- is generic
	- gives size explicitly

UINT64_C(0xffffffffffffffff) is better than UINT64_MAX because
	- rte_flow.h has a lot of bitmasks
	- it is copy/paste safe

Acked-by: Thomas Monjalon <thomas@monjalon.net>



  parent reply	other threads:[~2020-04-28  9:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27 13:23 [dpdk-dev] [PATCH 0/3] 20.05-rc1 fixes for OVS David Marchand
2020-04-27 13:23 ` [dpdk-dev] [PATCH 1/3] ring: fix build with -Wswitch-enum David Marchand
2020-04-27 14:55   ` Ananyev, Konstantin
2020-04-27 13:23 ` [dpdk-dev] [PATCH 2/3] eal: fix typo in endian conversion macros David Marchand
2020-04-27 13:35   ` Bruce Richardson
2020-04-27 13:23 ` [dpdk-dev] [PATCH 3/3] ethdev: fix build warning on 64-bit value David Marchand
2020-04-27 13:33   ` Andrew Rybchenko
2020-04-27 13:34   ` Bruce Richardson
2020-04-27 13:36     ` David Marchand
2020-04-27 13:46       ` Ananyev, Konstantin
2020-04-27 14:00         ` David Marchand
2020-04-27 14:07           ` Ferruh Yigit
2020-04-27 14:12             ` David Marchand
2020-04-27 14:14               ` Ferruh Yigit
2020-04-28  9:27           ` Thomas Monjalon [this message]
2020-05-06 14:37             ` Morten Brørup
2020-04-27 13:39     ` Andrew Rybchenko
2020-04-27 13:40       ` David Marchand
2020-04-27 13:43         ` Bruce Richardson
2020-04-28  9:53 ` [dpdk-dev] [PATCH 0/3] 20.05-rc1 fixes for OVS David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1847700.fxN4lLDhpz@thomas \
    --to=thomas@monjalon.net \
    --cc=arybchenko@solarflare.com \
    --cc=bruce.richardson@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=orika@mellanox.com \
    --cc=xiao.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.