Re: [PATCH] cpp: Fix missing extern "C" closing bracket

[Edvard Fagerholm <edvard.fagerholm@gmail.com>]

On Tue, Jun 24, 2025 at 4:53 PM Bruce Richardson
<bruce.richardson@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.richardson@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
> >