On Tue, Oct 26, 2021 at 09:45:20AM +0200, Morten Brørup wrote: > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Thomas Monjalon > > Sent: Monday, 25 October 2021 21.14 > > > > 15/03/2021 20:34, Tyler Retzlaff: > > > The proposal has resulted from request to review [1] the following > > > functions where there appeared to be inconsistency in return type > > > or parameter type selections for the following inline functions. > > > > > > rte_bsf32() > > > rte_bsf32_safe() > > > rte_bsf64() > > > rte_bsf64_safe() > > > rte_fls_u32() > > > rte_fls_u64() > > > rte_log2_u32() > > > rte_log2_u64() > > > > > > [1] http://mails.dpdk.org/archives/dev/2021-March/201590.html > > > > > > Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com> > > > --- > > > --- a/doc/guides/rel_notes/deprecation.rst > > > +++ b/doc/guides/rel_notes/deprecation.rst > > > +* eal: Fix inline function return and parameter types for > > rte_{bsf,fls} > > > + inline functions to be consistent. > > > + Change ``rte_bsf32_safe`` parameter ``v`` from ``uint64_t`` to > > ``uint32_t``. > > > + Change ``rte_bsf64`` return type to ``uint32_t`` instead of > > ``int``. > > > + Change ``rte_fls_u32`` return type to ``uint32_t`` instead of > > ``int``. > > > + Change ``rte_fls_u64`` return type to ``uint32_t`` instead of > > ``int``. > > > > It seems we completely forgot this. > > How critical is it? >
our organization as a matter of internal security policy requires these sorts of things to be fixed. while i didn't see any bugs in the dpdk code there is an opportunity for users of these functions to accidentally write code that is prone to integer and buffer overflow class bugs. there is no urgency, but why leave things sloppy? though i do wish this had been responded to in a more timely manner 7 months for something that should have almost been rubber stamped. > Not updating has near zero effect on bug probability: Incorrectly returning > signed int instead of unsigned int is extremely unlikely to cause problems. > > Updating has near zero performance improvement: The unnecessary expansion of > a parameter value from 32 to 64 bits is cheap. > > The update's only tangible benefit is API consistency. :-) > > -Morten
