07/07/2020 13:38, Parav Pandit: > From: Morten Brørup <m...@smartsharesystems.com> > > From: Parav Pandit > > > --- a/lib/librte_eal/include/rte_bitops.h > > > +++ b/lib/librte_eal/include/rte_bitops.h > > > @@ -17,6 +17,8 @@ > > > #include <rte_debug.h> > > > #include <rte_compat.h> > > > > > > +#define RTE_BIT(bit_num) (1UL << (bit_num)) > > > > Is the return value 32 or 64 bit, or is intended to depend on the target > > architecture? > > > It should be 64-bit. > > > Please be explicit by using UINT32_C(1) or UINT64_C(1) instead of 1UL, if > > you > > want a specific size. > > > Will do UINT64_C(1). > > > It could be a static inline __attribute__((__pure__)) function instead of a > > macro, > > but it's not important for me. > > > > The macro/function needs a description for the documentation. > > > In this header file or outside?
It is asked to add a doxygen comment. > > I'm also concerned about the name of the macro being too generic. But the > > effort of changing all the drivers where it is being used already could be > > too big > > if the name changes too. > > > Right. Currently drivers have generic name as BIT(). Close to 3000 entries. > So doing at RTE_BIT to match other rte_ APIs. > Drivers can slowly migrate at their pace to this one. > > > And the macro/function is new, so shouldn't it - in theory - be marked as > > experimental? > > How to mark a macro as experimental? A macro cannot be experimental.
