> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Tyler Retzlaff > Sent: Friday, March 12, 2021 7:24 PM > > On Fri, Mar 12, 2021 at 08:34:50AM +0100, Morten Brørup wrote: > > CC: ABI Policy maintainers. You might have an opinion. Or not. :-) > > > > > > Please also update the similar math functions in rte_common.h, so the > return type is consistent across these functions: > > - rte_bsf32() > > - rte_bsf32_safe() > > - rte_fls_u32() > > - rte_bsf64() > > - rte_fls_u64() > > - rte_log2_u32() > > - rte_log2_u64() > > agreed, happy to review the whole set and deal with it all at once.
Ups. I should have omitted rte_bsf32_safe() from the list. It returns a Boolean. > > > > > They should all return either int or uint32_t. > > > > Standard C conventions would have them all return int (probably due to > C's default type promotion to int when used in calculations), which is also > the type returned by their underlying implementation. > > yes, i suspect gcc builtins return int because of the default type > promotion. probably historical be interesting to get an old gcc hand to > tell us a story. > > > > > For some unknown reason, DPDK often uses uint32_t where you would > normally use int. I guess it was inspired by MISRA C (for embedded > systems); but it is not a documented conventions, and often deviated from. > > horses for courses, if it doesn't make sense to interpret as signed then > i don't see a lot of value in using signed and it can open up avenues of > exploit. I agree with you on this. The best return type is determined by considering how the return value is going to be used. I could argue that these are mathematical functions, so they can be used for any kind of math, including math involving negative numbers. On the other hand, DPDK generally uses uint32_t for positive integers; and this also seems to be the original author's intention. > > > > > I don't have a personal preference for int or uint32_t here. But at least > follow the same convention in the same header file. > > agree completely, consistency. Looking closer at it, uint32_t is probably closer to general DPDK consistency. > > > > > (Please note that the functions returning a Boolean value as an int type > should keep doing that.) > > i'm not planning on changing int -> _Bool. but i am curious about your > comment. stdbool.h is already used in the code base is there a compiler > in use that does not support _Bool. this is purely my interest, i don't > propose any change. I only mentioned this to ensure that you don't change the Boolean return values from int to uint32_t. For arguments sate, they could be changed to bool, which is an acceptable type in DPDK, ref.: https://elixir.bootlin.com/dpdk/latest/C/ident/bool However, I agree with you to not propose any change here!
