From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 02926A056A; Wed, 10 Mar 2021 23:52:40 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7469522A546; Wed, 10 Mar 2021 23:52:40 +0100 (CET) Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mails.dpdk.org (Postfix) with ESMTP id EDAFB22A441 for ; Wed, 10 Mar 2021 23:52:38 +0100 (CET) Received: by linux.microsoft.com (Postfix, from userid 1086) id 2FE5620B39C5; Wed, 10 Mar 2021 14:52:38 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 2FE5620B39C5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1615416758; bh=JfzDf62qRPm5hKqjgpkyxCSJ+30a6KaSGgkl3TgYtSQ=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=OMQMbethbiDUkpL2bpbQK84kAoVMGjJpwgIZ/HPgO/1NS30d2rVyCwmSNPOiFKkKh mT6I7xOcnX3t9bziVvd+5zZeV8V21YMbbDNbc8Ky897rKe5uDJzNzcoo+izYp79yqP wGVoUS/X2R1mJIcefUaVjS8XFlbmTTTM38avVtr8= Date: Wed, 10 Mar 2021 14:52:38 -0800 From: Tyler Retzlaff To: Stephen Hemminger Cc: dev@dpdk.org, anatoly.burakov@intel.com Message-ID: <20210310225238.GA10267@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <1615358466-12761-1-git-send-email-roretzla@linux.microsoft.com> <20210310104942.66ef440e@hermes.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210310104942.66ef440e@hermes.local> User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [dpdk-dev] [PATCH] librte_eal/common: fix return type of rte_bsf64 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Mar 10, 2021 at 10:49:42AM -0800, Stephen Hemminger wrote: > On Tue, 9 Mar 2021 22:41:06 -0800 > Tyler Retzlaff wrote: > > > based on the original commit and the usage of rte_bsf64 it appears the > > function should always have returned uint32_t instead of int which is > > consistent with the cast introduced in the return statement. > > > > Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf > > functions") > > Cc: anatoly.burakov@intel.com > > > > Signed-off-by: Tyler Retzlaff > > --- > > lib/librte_eal/include/rte_common.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/librte_eal/include/rte_common.h b/lib/librte_eal/include/rte_common.h > > index 1b630baf1..5e70ee7a8 100644 > > --- a/lib/librte_eal/include/rte_common.h > > +++ b/lib/librte_eal/include/rte_common.h > > @@ -679,7 +679,7 @@ rte_fls_u32(uint32_t x) > > * @return > > * least significant set bit in the input parameter. > > */ > > -static inline int > > +static inline uint32_t > > rte_bsf64(uint64_t v) > > { > > return (uint32_t)__builtin_ctzll(v); > > The cast is no longer needed, it should be removed. it's not so much about making it compile. it's about making it correct with respect to original author intent, consistent with the rte_bsf32 declaration, other consumers of the inline function inside rte_common.h and whether or not it makes sense to have a function that returns a count of bits signed. based on those factors i'm asserting that the cast is actually correct and it is the return type that is wrong. your suggestion however would avoid having to deal with the downside of changing the return type which is that an api change is necessary since the function is exposed to applications. but even for something small like this i think it is best to pursue the correct change rather than sprinkle casts to (uint32_t) at various call-sites. i'm in the process of sending the proposal to deprecate/change the return type unless others feel the above evaluation missed the mark. thanks!