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

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

[Edvard Fagerholm]

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.
---
 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

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

[Bruce Richardson]

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

>  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
> 

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

[Edvard Fagerholm]

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
> >

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

[Edvard Fagerholm]

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.

Signed-off-by: Edvard Fagerholm <edvard.fagerholm@gmail.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/eal/x86/include/rte_atomic.h    | 6 +++---
 lib/eal/x86/include/rte_byteorder.h | 9 +++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/lib/eal/x86/include/rte_atomic.h b/lib/eal/x86/include/rte_atomic.h
index c72c47c83e..7ba1722683 100644
--- a/lib/eal/x86/include/rte_atomic.h
+++ b/lib/eal/x86/include/rte_atomic.h
@@ -282,18 +282,18 @@ static inline int rte_atomic32_dec_and_test(rte_atomic32_t *v)
 	return ret != 0;
 }
 
+#endif /* RTE_FORCE_INTRINSICS */
+
 #ifdef __cplusplus
 }
 #endif
 
-#endif
-
 #ifdef RTE_ARCH_I686
 #include "rte_atomic_32.h"
 #else
 #include "rte_atomic_64.h"
 #endif
 
-#endif
+#endif /* RTE_TOOLCHAIN_MSVC */
 
 #endif /* _RTE_ATOMIC_X86_H_ */
diff --git a/lib/eal/x86/include/rte_byteorder.h b/lib/eal/x86/include/rte_byteorder.h
index 5a49ffcd50..f54bb07cd0 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 /* RTE_FORCE_INTRINSICS */
+
+#ifdef __cplusplus
+}
 #endif
 
 #define rte_cpu_to_le_16(x) (x)
-- 
2.34.1