[PATCH v6 1/6] eal: provide rte stdatomics optional atomics API
Morten Brørup
mb at smartsharesystems.com
Fri Sep 29 10:54:24 CEST 2023
> From: David Marchand [mailto:david.marchand at redhat.com]
> Sent: Friday, 29 September 2023 10.04
>
> On Thu, Sep 28, 2023 at 10:06 AM Thomas Monjalon <thomas at monjalon.net> wrote:
> >
> > 22/08/2023 23:00, Tyler Retzlaff:
[...]
> > > +/* The memory order is an enumerated type in C11. */
> > > +typedef memory_order rte_memory_order;
> > > +
> > > +#define rte_memory_order_relaxed memory_order_relaxed
> > > +#ifdef __ATOMIC_RELAXED
> > > +static_assert(rte_memory_order_relaxed == __ATOMIC_RELAXED,
> > > + "rte_memory_order_relaxed == __ATOMIC_RELAXED");
> >
> > Not sure about using static_assert or RTE_BUILD_BUG_ON
>
> Do you mean you want no check at all in a public facing header?
>
> Or is it that we have RTE_BUILD_BUG_ON and we should keep using it
> instead of static_assert?
>
> I remember some problems with RTE_BUILD_BUG_ON where the compiler
> would silently drop the whole expression and reported no problem as it
> could not evaluate the expression.
> At least, with static_assert (iirc, it is new to C11) the compiler
> complains with a clear "error: expression in static assertion is not
> constant".
> We could fix RTE_BUILD_BUG_ON, but I guess the fix would be equivalent
> to map it to static_assert(!condition).
> Using language standard constructs seems a better choice.
+1 to using language standard constructs.
static_assert became standard in C11. (Formally, _Static_assert is standard C11, and static_assert is available through a convenience macro in C11 [1].)
In my opinion, our move to C11 thus makes RTE_BUILD_BUG_ON obsolete.
We should mark RTE_BUILD_BUG_ON as deprecated, and disallow RTE_BUILD_BUG_ON in new code. Perhaps checkpatches could catch this?
[1]: https://en.cppreference.com/w/c/language/_Static_assert
PS: static_assert also has the advantage that it can be used directly in header files. RTE_BUILD_BUG_ON can only be used in functions, and thus needs to be wrapped in a dummy (static inline) function when used in a header file.
>
>
> --
> David Marchand
More information about the dev
mailing list