> From: Bruce Richardson [mailto:bruce.richard...@intel.com] > Sent: Wednesday, 17 January 2024 10.27 > > On Wed, Jan 17, 2024 at 10:58:12AM +0300, Andrew Rybchenko wrote: > > On 1/16/24 21:41, Stephen Hemminger wrote: > > > RTE_BUILD_BUG_ON() was being used with a non-constant value. > > > The inline function rte_is_power_of_2() is not constant since > > > inline expansion happens later in the compile process. > > > Replace it with macro which will be constant. > > > > > > Fixes: 4236ce9bf5bf ("event/opdl: add OPDL ring infrastructure > library") > > > Cc: liang.j...@intel.com > > > Signed-off-by: Stephen Hemminger <step...@networkplumber.org> > > > Acked-by: Bruce Richardson <bruce.richard...@intel.com> > > > Acked-by: Tyler Retzlaff <roret...@linux.microsoft.com> > > > --- > > > drivers/event/opdl/opdl_ring.c | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/event/opdl/opdl_ring.c > b/drivers/event/opdl/opdl_ring.c > > > index 69392b56bbec..24e0bbe3222d 100644 > > > --- a/drivers/event/opdl/opdl_ring.c > > > +++ b/drivers/event/opdl/opdl_ring.c > > > @@ -31,6 +31,9 @@ > > > #define OPDL_OPA_MASK (0xFF) > > > #define OPDL_OPA_OFFSET (0x38) > > > +/* Equivalent to rte_is_power_of_2() but as macro. */ > > > +#define IS_POWER_OF_2(x) (((x) & ((x) - 1)) == 0) > > > > IMHO adding it in specific driver is a wrong direction. I'm afraid it > > will result in duplication of such macros in code base without clear > > reason why. > > > > May be it is better to add it with a proper name to EAL? > > > +1 > Even if it's a lower-case name, lets make it a macro in EAL so we can > use > it everywhere in asserts.
Here's a thought... we could introduce a new code convention for this purpose. E.g. either: 1. Macros in lower case are guaranteed to access each parameter exactly once (and might not be usable for static_assert; they somewhat resemble inline functions), and macros in upper case may access parameters any number of times (and must be usable for static_assert). Or: 2. Macros that access any parameter zero or multiple times must have their name postfixed _UNSAFE (or some other word).
