diff mbox series

[v3] eal: fix argument to rte_bsf32_safe

Message ID 20210723154546.7958-1-stephen@networkplumber.org (mailing list archive)
State Accepted, archived
Headers show
Series [v3] eal: fix argument to rte_bsf32_safe | expand

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot success github build: passed
ci/intel-Testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/checkpatch warning coding style issues

Commit Message

Stephen Hemminger July 23, 2021, 3:45 p.m. UTC
The first argument to rte_bsf32_safe was incorrectly declared as
a 64 bit value. The code only works on 32 bit values and the underlying
function rte_bsf32 only accepts 32 bit values. This was a mistake
introduced when the safe version was added and probably cause
by copy/paste from the 64 bit version.

The bug passed silently under the radar until some other code was
built with -Wall and -Wextra in C++ and C++ complains about the
missing cast.

Yes, this is a API signature change, but the original code was wrong.
It is an inline so not an ABI change.

Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
Cc: anatoly.burakov@intel.com
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
v3 - reword commit description for checkpatch

 doc/guides/rel_notes/release_21_08.rst | 4 ++++
 lib/eal/include/rte_common.h           | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

Comments

Thomas Monjalon July 24, 2021, 7:58 a.m. UTC | #1
23/07/2021 17:45, Stephen Hemminger:
> The first argument to rte_bsf32_safe was incorrectly declared as
> a 64 bit value. The code only works on 32 bit values and the underlying
> function rte_bsf32 only accepts 32 bit values. This was a mistake
> introduced when the safe version was added and probably cause
> by copy/paste from the 64 bit version.
> 
> The bug passed silently under the radar until some other code was
> built with -Wall and -Wextra in C++ and C++ complains about the
> missing cast.
> 
> Yes, this is a API signature change, but the original code was wrong.
> It is an inline so not an ABI change.
> 
> Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
> Cc: anatoly.burakov@intel.com
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

+Cc: stable@dpdk.org

Applied, thanks.

I think these functions lack a reference to the name Bit Scan Forward.
Stephen Hemminger July 24, 2021, 11:50 p.m. UTC | #2
On Sat, 24 Jul 2021 09:58:44 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 23/07/2021 17:45, Stephen Hemminger:
> > The first argument to rte_bsf32_safe was incorrectly declared as
> > a 64 bit value. The code only works on 32 bit values and the underlying
> > function rte_bsf32 only accepts 32 bit values. This was a mistake
> > introduced when the safe version was added and probably cause
> > by copy/paste from the 64 bit version.
> > 
> > The bug passed silently under the radar until some other code was
> > built with -Wall and -Wextra in C++ and C++ complains about the
> > missing cast.
> > 
> > Yes, this is a API signature change, but the original code was wrong.
> > It is an inline so not an ABI change.
> > 
> > Fixes: 4e261f551986 ("eal: add 64-bit bsf and 32-bit safe bsf functions")
> > Cc: anatoly.burakov@intel.com
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>  
> 
> +Cc: stable@dpdk.org
> 
> Applied, thanks.
> 
> I think these functions lack a reference to the name Bit Scan Forward.
> 
> 
> 
> 

Tyler wanted to fix a bunch more stuff in these for 21.11 where it will
be a bigger API change.
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_21_08.rst b/doc/guides/rel_notes/release_21_08.rst
index e2c5ccbf7d90..148405891fcb 100644
--- a/doc/guides/rel_notes/release_21_08.rst
+++ b/doc/guides/rel_notes/release_21_08.rst
@@ -196,6 +196,10 @@  API Changes
   to be thread safe; all Rx queues affected by the API will now need to be
   stopped before making any changes to the power management scheme.
 
+* eal: ``rte_bsf32_safe`` now takes a 32 bit value for its first
+  argument. This fixes warnings about loss of precision when used
+  with some compilers settings.
+
 
 ABI Changes
 -----------
diff --git a/lib/eal/include/rte_common.h b/lib/eal/include/rte_common.h
index d5a32c66a5fe..99eb5f1820ae 100644
--- a/lib/eal/include/rte_common.h
+++ b/lib/eal/include/rte_common.h
@@ -623,7 +623,7 @@  rte_bsf32(uint32_t v)
  *     Returns 0 if ``v`` was 0, otherwise returns 1.
  */
 static inline int
-rte_bsf32_safe(uint64_t v, uint32_t *pos)
+rte_bsf32_safe(uint32_t v, uint32_t *pos)
 {
 	if (v == 0)
 		return 0;