On Tue, Jun 24, 2025 at 4:53 PM Bruce Richardson <bruce.richard...@intel.com> wrote: > > On Tue, Jun 24, 2025 at 02:12:53PM +0300, Edvard Fagerholm wrote: > > RTE_FORCE_INTRINSICS with a C++ compiler causes rte_atomic.h and > > rte_byteorder.h to emit on x86 an extern "C" without a closing > > brace. > > --- > > Unfortunately, patch is missing a signoff, so isn't eligible for merge. Can > you please submit a v2 with the appropriate signoff included. See [1] for > more details. > > For the patch itself, it looks ok to me, but really the various #endif and > #else tags need comments on them to indicate which #ifdef they belong to > (unless it's a very short ifdef). If you do a v2 would you consider adding > those comments to help make the code clearer? > > With or without that suggestion, you can include my ack on v2. > > Acked-by: Bruce Richardson <bruce.richard...@intel.com> > > Regards, > /Bruce > > [1] https://doc.dpdk.org/guides/contributing/patches.html
Thanks. I'll make another patch with the signoff and add the comments. Best, Edvard > > > lib/eal/x86/include/rte_atomic.h | 4 ++-- > > lib/eal/x86/include/rte_byteorder.h | 9 +++++---- > > 2 files changed, 7 insertions(+), 6 deletions(-) > > > > diff --git a/lib/eal/x86/include/rte_atomic.h > > b/lib/eal/x86/include/rte_atomic.h > > index c72c47c83e..68629016b4 100644 > > --- a/lib/eal/x86/include/rte_atomic.h > > +++ b/lib/eal/x86/include/rte_atomic.h > > @@ -282,10 +282,10 @@ static inline int > > rte_atomic32_dec_and_test(rte_atomic32_t *v) > > return ret != 0; > > } > > > > -#ifdef __cplusplus > > -} > > #endif > > > > +#ifdef __cplusplus > > +} > > #endif > > > > #ifdef RTE_ARCH_I686 > > diff --git a/lib/eal/x86/include/rte_byteorder.h > > b/lib/eal/x86/include/rte_byteorder.h > > index 5a49ffcd50..62a87c1aa6 100644 > > --- a/lib/eal/x86/include/rte_byteorder.h > > +++ b/lib/eal/x86/include/rte_byteorder.h > > @@ -48,10 +48,6 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x) > > return x; > > } > > > > -#ifdef __cplusplus > > -} > > -#endif > > - > > #define rte_bswap16(x) ((uint16_t)(__builtin_constant_p(x) ? \ > > rte_constant_bswap16(x) : \ > > rte_arch_bswap16(x))) > > @@ -69,6 +65,11 @@ static inline uint32_t rte_arch_bswap32(uint32_t _x) > > #else > > #include "rte_byteorder_64.h" > > #endif > > + > > +#endif > > + > > +#ifdef __cplusplus > > +} > > #endif > > > > #define rte_cpu_to_le_16(x) (x) > > -- > > 2.34.1 > >
