> From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com] > Sent: Monday, July 31, 2017 5:18 PM > To: Van Haaren, Harry <harry.van.haa...@intel.com> > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH] service: fix shifts to operate on 64 bit > integers > > Hi Harry, > > On Mon, Jul 31, 2017 at 04:58:27PM +0100, Harry van Haaren wrote: > > This commit fixes shifts to an integer (1 << shift) which > > is assumed to be a 32-bit integer. In this case, the shift is > > variable and expected to be valid for 64-bit integers. Given that > > the expectation to work with 64 bits exists, we must ensure that > > the (1 << shift) one in that formula is actually a uin64_t. > > > > Simply defining a const uint64_t and using it ensures the compiler > > is aware of the intention. The issue would only manifest if there > > were greater than 31 services registered. > > > > Fixes: 21698354c832 ("service: introduce service cores concept") > > > > Signed-off-by: Harry van Haaren <harry.van.haa...@intel.com> > > --- > > lib/librte_eal/common/rte_service.c | 16 ++++++++++------ > > 1 file changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/lib/librte_eal/common/rte_service.c > > b/lib/librte_eal/common/rte_service.c > > index e82b9ad..8c1cffa 100644 > > --- a/lib/librte_eal/common/rte_service.c > > +++ b/lib/librte_eal/common/rte_service.c > > @@ -285,8 +285,9 @@ rte_service_unregister(struct rte_service_spec *spec) > > > > s->internal_flags &= ~(SERVICE_F_REGISTERED); > > > > + const uint64_t one = 1; > > for (i = 0; i < RTE_MAX_LCORE; i++) > > - lcore_states[i].service_mask &= ~(1 << service_id); > > + lcore_states[i].service_mask &= ~(one << service_id); > > Why not use UINT64_C(1)?
Mostly because I've never heard of it before :) Thanks for review, still learnin' every day! Sending v2's every other day...
