[v3,1/1] eal: add C++ include guard in generic/rte_vect.h
Checks
Commit Message
The header was missing the extern "C" directive which causes name
mangling of functions by C++ compilers, leading to linker errors
complaining of undefined references to these functions.
Also updated the coding style contribution guideline with a new
"Language Linkage" section stating `extern "C"` block should be added to
public headers.
Fixes: 86c743cf9140 ("eal: define generic vector types")
Cc: nelio.laranjeiro@6wind.com
Cc: stable@dpdk.org
Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
---
.mailmap | 2 +-
doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
lib/eal/include/generic/rte_vect.h | 8 ++++++++
3 files changed, 30 insertions(+), 1 deletion(-)
Comments
Hi everyone,
I've made the updates as suggested. Could someone please review the latest
patchset? Not sure if I followed the new patchset instructions correctly,
I've always had trouble with that part.
Thanks,
Ashish.
On Sun, Mar 17, 2024 at 8:44 PM Ashish Sadanandan <
ashish.sadanandan@gmail.com> wrote:
> The header was missing the extern "C" directive which causes name
> mangling of functions by C++ compilers, leading to linker errors
> complaining of undefined references to these functions.
>
> Also updated the coding style contribution guideline with a new
> "Language Linkage" section stating `extern "C"` block should be added to
> public headers.
>
> Fixes: 86c743cf9140 ("eal: define generic vector types")
> Cc: nelio.laranjeiro@6wind.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> ---
> .mailmap | 2 +-
> doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
> lib/eal/include/generic/rte_vect.h | 8 ++++++++
> 3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/.mailmap b/.mailmap
> index 50726e1232..24de59ba89 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -142,7 +142,7 @@ Ashijeet Acharya <ashijeet.acharya@6wind.com>
> Ashish Gupta <ashishg@marvell.com> <ashish.gupta@marvell.com> <
> ashish.gupta@caviumnetworks.com>
> Ashish Jain <ashish.jain@nxp.com>
> Ashish Paul <apaul@juniper.net>
> -Ashish Sadanandan <ashish.sadanandan@gmail.com>
> +Ashish Sadanandan <ashish.sadanandan@gmail.com> <
> quic_asadanan@quicinc.com>
> Ashish Shah <ashish.n.shah@intel.com>
> Ashwin Sekhar T K <asekhar@marvell.com> <ashwin.sekhar@caviumnetworks.com
> >
> Asim Jamshed <asim.jamshed@gmail.com>
> diff --git a/doc/guides/contributing/coding_style.rst
> b/doc/guides/contributing/coding_style.rst
> index 1ebc79ca3c..27c947fcec 100644
> --- a/doc/guides/contributing/coding_style.rst
> +++ b/doc/guides/contributing/coding_style.rst
> @@ -106,6 +106,27 @@ Headers should be protected against multiple
> inclusion with the usual:
>
> #endif /* _FILE_H_ */
>
> +Language Linkage
> +~~~~~~~~~~~~~~~~
> +
> +Public headers should enclose all function and variable declarations and
> definitions in an ``extern "C"`` block to facilitate interoperability with
> C++.
> +
> +.. code-block:: c
> +
> + #ifndef _FILE_H_
> + #define _FILE_H_
> +
> + #ifdef __cplusplus
> + extern "C" {
> + #endif
> +
> + /* Code */
> +
> + #ifdef __cplusplus
> + }
> + #endif
> +
> + #endif /* _FILE_H_ */
>
> Macros
> ~~~~~~
> diff --git a/lib/eal/include/generic/rte_vect.h
> b/lib/eal/include/generic/rte_vect.h
> index 6540419cd2..3578d8749b 100644
> --- a/lib/eal/include/generic/rte_vect.h
> +++ b/lib/eal/include/generic/rte_vect.h
> @@ -15,6 +15,10 @@
>
> #include <stdint.h>
>
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> #ifndef RTE_TOOLCHAIN_MSVC
>
> /* Unsigned vector types */
> @@ -226,4 +230,8 @@ uint16_t rte_vect_get_max_simd_bitwidth(void);
> */
> int rte_vect_set_max_simd_bitwidth(uint16_t bitwidth);
>
> +#ifdef __cplusplus
> +}
> +#endif
> +
> #endif /* _RTE_VECT_H_ */
> --
> 2.31.1
>
>
On Sun, Mar 17, 2024 at 08:44:15PM -0600, Ashish Sadanandan wrote:
> The header was missing the extern "C" directive which causes name
> mangling of functions by C++ compilers, leading to linker errors
> complaining of undefined references to these functions.
>
> Also updated the coding style contribution guideline with a new
> "Language Linkage" section stating `extern "C"` block should be added to
> public headers.
>
> Fixes: 86c743cf9140 ("eal: define generic vector types")
> Cc: nelio.laranjeiro@6wind.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
On Sun, Mar 17, 2024 at 08:44:15PM -0600, Ashish Sadanandan wrote:
> The header was missing the extern "C" directive which causes name
> mangling of functions by C++ compilers, leading to linker errors
> complaining of undefined references to these functions.
>
> Also updated the coding style contribution guideline with a new
> "Language Linkage" section stating `extern "C"` block should be added to
> public headers.
>
> Fixes: 86c743cf9140 ("eal: define generic vector types")
> Cc: nelio.laranjeiro@6wind.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> ---
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
02/04/2024 18:03, Ashish Sadanandan:
> Hi everyone,
> I've made the updates as suggested. Could someone please review the latest
> patchset? Not sure if I followed the new patchset instructions correctly,
> I've always had trouble with that part.
I remember we were discussing about aligning all files.
I was waiting for a patch applying the rule we discussed.
> On Sun, Mar 17, 2024 at 8:44 PM Ashish Sadanandan <
> ashish.sadanandan@gmail.com> wrote:
>
> > The header was missing the extern "C" directive which causes name
> > mangling of functions by C++ compilers, leading to linker errors
> > complaining of undefined references to these functions.
> >
> > Also updated the coding style contribution guideline with a new
> > "Language Linkage" section stating `extern "C"` block should be added to
> > public headers.
> >
> > Fixes: 86c743cf9140 ("eal: define generic vector types")
> > Cc: nelio.laranjeiro@6wind.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> > ---
> > .mailmap | 2 +-
> > doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
> > lib/eal/include/generic/rte_vect.h | 8 ++++++++
> > 3 files changed, 30 insertions(+), 1 deletion(-)
> >
> > --- a/doc/guides/contributing/coding_style.rst
> > +++ b/doc/guides/contributing/coding_style.rst
> > +Language Linkage
> > +~~~~~~~~~~~~~~~~
> > +
> > +Public headers should enclose all function and variable declarations and
> > definitions in an ``extern "C"`` block to facilitate interoperability with
> > C++.
> > +
> > +.. code-block:: c
> > +
> > + #ifndef _FILE_H_
> > + #define _FILE_H_
> > +
> > + #ifdef __cplusplus
> > + extern "C" {
> > + #endif
> > +
> > + /* Code */
> > +
> > + #ifdef __cplusplus
> > + }
> > + #endif
> > +
> > + #endif /* _FILE_H_ */
This is not describing where the includes should be placed.
On Wed, Apr 3, 2024 at 8:52 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> 02/04/2024 18:03, Ashish Sadanandan:
> > Hi everyone,
> > I've made the updates as suggested. Could someone please review the
> latest
> > patchset? Not sure if I followed the new patchset instructions correctly,
> > I've always had trouble with that part.
>
> I remember we were discussing about aligning all files.
> I was waiting for a patch applying the rule we discussed.
>
> I missed the part where people were volunteering me for additional work :)
The consensus seems to be that the extern "C" directives should only be in
public headers, not private ones. Can you please tell me if there's an easy
way to get a list of public headers?
> > On Sun, Mar 17, 2024 at 8:44 PM Ashish Sadanandan <
> > ashish.sadanandan@gmail.com> wrote:
> >
> > > The header was missing the extern "C" directive which causes name
> > > mangling of functions by C++ compilers, leading to linker errors
> > > complaining of undefined references to these functions.
> > >
> > > Also updated the coding style contribution guideline with a new
> > > "Language Linkage" section stating `extern "C"` block should be added
> to
> > > public headers.
> > >
> > > Fixes: 86c743cf9140 ("eal: define generic vector types")
> > > Cc: nelio.laranjeiro@6wind.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> > > ---
> > > .mailmap | 2 +-
> > > doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
> > > lib/eal/include/generic/rte_vect.h | 8 ++++++++
> > > 3 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > --- a/doc/guides/contributing/coding_style.rst
> > > +++ b/doc/guides/contributing/coding_style.rst
> > > +Language Linkage
> > > +~~~~~~~~~~~~~~~~
> > > +
> > > +Public headers should enclose all function and variable declarations
> and
> > > definitions in an ``extern "C"`` block to facilitate interoperability
> with
> > > C++.
> > > +
> > > +.. code-block:: c
> > > +
> > > + #ifndef _FILE_H_
> > > + #define _FILE_H_
> > > +
> > > + #ifdef __cplusplus
> > > + extern "C" {
> > > + #endif
> > > +
> > > + /* Code */
> > > +
> > > + #ifdef __cplusplus
> > > + }
> > > + #endif
> > > +
> > > + #endif /* _FILE_H_ */
>
> This is not describing where the includes should be placed.
>
Will address this along with the rest of the headers.
Regards,
Ashish.
On Sat, 6 Apr 2024 19:30:38 -0600
Ashish Sadanandan <ashish.sadanandan@gmail.com> wrote:
> On Wed, Apr 3, 2024 at 8:52 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 02/04/2024 18:03, Ashish Sadanandan:
> > > Hi everyone,
> > > I've made the updates as suggested. Could someone please review the
> > latest
> > > patchset? Not sure if I followed the new patchset instructions correctly,
> > > I've always had trouble with that part.
> >
> > I remember we were discussing about aligning all files.
> > I was waiting for a patch applying the rule we discussed.
> >
> > I missed the part where people were volunteering me for additional work :)
>
> The consensus seems to be that the extern "C" directives should only be in
> public headers, not private ones. Can you please tell me if there's an easy
> way to get a list of public headers?
The issue for private headers that Tyler mentioned is that MS has private
DPDK PMD's that are written in C++. Not sure if this needs to be considered
or supported.
On 4/7/2024 2:30 AM, Ashish Sadanandan wrote:
>
>
> On Wed, Apr 3, 2024 at 8:52 AM Thomas Monjalon <thomas@monjalon.net
> <mailto:thomas@monjalon.net>> wrote:
>
> 02/04/2024 18:03, Ashish Sadanandan:
> > Hi everyone,
> > I've made the updates as suggested. Could someone please review
> the latest
> > patchset? Not sure if I followed the new patchset instructions
> correctly,
> > I've always had trouble with that part.
>
> I remember we were discussing about aligning all files.
> I was waiting for a patch applying the rule we discussed.
>
> I missed the part where people were volunteering me for additional work :)
>
> The consensus seems to be that the extern "C" directives should only be
> in public headers, not private ones. Can you please tell me if there's
> an easy way to get a list of public headers?
>
Public headers are installed as part of ninja install target, so one
option is you can install a build to a custom folder and get the list
from there.
Or they are listed in meson.build files in 'headers' variable, you can
parse them.
Don't get confused with 'driver_sdk_headers' variable in meson files,
that is the list of headers required for driver development, and
installed if sdk ('enable_driver_sdk') meson option is enabled.
>
> > On Sun, Mar 17, 2024 at 8:44 PM Ashish Sadanandan <
> > ashish.sadanandan@gmail.com <mailto:ashish.sadanandan@gmail.com>>
> wrote:
> >
> > > The header was missing the extern "C" directive which causes name
> > > mangling of functions by C++ compilers, leading to linker errors
> > > complaining of undefined references to these functions.
> > >
> > > Also updated the coding style contribution guideline with a new
> > > "Language Linkage" section stating `extern "C"` block should be
> added to
> > > public headers.
> > >
> > > Fixes: 86c743cf9140 ("eal: define generic vector types")
> > > Cc: nelio.laranjeiro@6wind.com <mailto:nelio.laranjeiro@6wind.com>
> > > Cc: stable@dpdk.org <mailto:stable@dpdk.org>
> > >
> > > Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com
> <mailto:ashish.sadanandan@gmail.com>>
> > > ---
> > > .mailmap | 2 +-
> > > doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
> > > lib/eal/include/generic/rte_vect.h | 8 ++++++++
> > > 3 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > --- a/doc/guides/contributing/coding_style.rst
> > > +++ b/doc/guides/contributing/coding_style.rst
> > > +Language Linkage
> > > +~~~~~~~~~~~~~~~~
> > > +
> > > +Public headers should enclose all function and variable
> declarations and
> > > definitions in an ``extern "C"`` block to facilitate
> interoperability with
> > > C++.
> > > +
> > > +.. code-block:: c
> > > +
> > > + #ifndef _FILE_H_
> > > + #define _FILE_H_
> > > +
> > > + #ifdef __cplusplus
> > > + extern "C" {
> > > + #endif
> > > +
> > > + /* Code */
> > > +
> > > + #ifdef __cplusplus
> > > + }
> > > + #endif
> > > +
> > > + #endif /* _FILE_H_ */
>
> This is not describing where the includes should be placed.
>
>
> Will address this along with the rest of the headers.
>
> Regards,
> Ashish.
On Mon, Apr 08, 2024 at 09:50:51AM +0100, Ferruh Yigit wrote:
> On 4/7/2024 2:30 AM, Ashish Sadanandan wrote:
> >
> >
> > On Wed, Apr 3, 2024 at 8:52 AM Thomas Monjalon <thomas@monjalon.net
> > <mailto:thomas@monjalon.net>> wrote:
> >
> > 02/04/2024 18:03, Ashish Sadanandan:
> > > Hi everyone,
> > > I've made the updates as suggested. Could someone please review
> > the latest
> > > patchset? Not sure if I followed the new patchset instructions
> > correctly,
> > > I've always had trouble with that part.
> >
> > I remember we were discussing about aligning all files.
> > I was waiting for a patch applying the rule we discussed.
> >
> > I missed the part where people were volunteering me for additional work :)
> >
> > The consensus seems to be that the extern "C" directives should only be
> > in public headers, not private ones. Can you please tell me if there's
> > an easy way to get a list of public headers?
> >
>
> Public headers are installed as part of ninja install target, so one
> option is you can install a build to a custom folder and get the list
> from there.
> Or they are listed in meson.build files in 'headers' variable, you can
> parse them.
>
>
> Don't get confused with 'driver_sdk_headers' variable in meson files,
> that is the list of headers required for driver development, and
> installed if sdk ('enable_driver_sdk') meson option is enabled.
>
I still don't see what the major issue is with just adding the guards to
all headers in DPDK. It would avoid any complications of which headers get
installed or when. They're stripped out on pre-processing, so it's not like
they are going to have a performance impact or anything.
/Bruce
On Sat, Apr 06, 2024 at 07:30:38PM -0600, Ashish Sadanandan wrote:
> On Wed, Apr 3, 2024 at 8:52 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 02/04/2024 18:03, Ashish Sadanandan:
> > > Hi everyone,
> > > I've made the updates as suggested. Could someone please review the
> > latest
> > > patchset? Not sure if I followed the new patchset instructions correctly,
> > > I've always had trouble with that part.
> >
> > I remember we were discussing about aligning all files.
> > I was waiting for a patch applying the rule we discussed.
> >
> > I missed the part where people were volunteering me for additional work :)
>
> The consensus seems to be that the extern "C" directives should only be in
> public headers, not private ones. Can you please tell me if there's an easy
> way to get a list of public headers?
build with driver_sdk_headers enabled.
run the meson install target.
all headers that are installed need the guard.
>
>
> > > On Sun, Mar 17, 2024 at 8:44 PM Ashish Sadanandan <
> > > ashish.sadanandan@gmail.com> wrote:
> > >
> > > > The header was missing the extern "C" directive which causes name
> > > > mangling of functions by C++ compilers, leading to linker errors
> > > > complaining of undefined references to these functions.
> > > >
> > > > Also updated the coding style contribution guideline with a new
> > > > "Language Linkage" section stating `extern "C"` block should be added
> > to
> > > > public headers.
> > > >
> > > > Fixes: 86c743cf9140 ("eal: define generic vector types")
> > > > Cc: nelio.laranjeiro@6wind.com
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Ashish Sadanandan <ashish.sadanandan@gmail.com>
> > > > ---
> > > > .mailmap | 2 +-
> > > > doc/guides/contributing/coding_style.rst | 21 +++++++++++++++++++++
> > > > lib/eal/include/generic/rte_vect.h | 8 ++++++++
> > > > 3 files changed, 30 insertions(+), 1 deletion(-)
> > > >
> > > > --- a/doc/guides/contributing/coding_style.rst
> > > > +++ b/doc/guides/contributing/coding_style.rst
> > > > +Language Linkage
> > > > +~~~~~~~~~~~~~~~~
> > > > +
> > > > +Public headers should enclose all function and variable declarations
> > and
> > > > definitions in an ``extern "C"`` block to facilitate interoperability
> > with
> > > > C++.
> > > > +
> > > > +.. code-block:: c
> > > > +
> > > > + #ifndef _FILE_H_
> > > > + #define _FILE_H_
> > > > +
> > > > + #ifdef __cplusplus
> > > > + extern "C" {
> > > > + #endif
> > > > +
> > > > + /* Code */
> > > > +
> > > > + #ifdef __cplusplus
> > > > + }
> > > > + #endif
> > > > +
> > > > + #endif /* _FILE_H_ */
> >
> > This is not describing where the includes should be placed.
> >
>
> Will address this along with the rest of the headers.
>
> Regards,
> Ashish.
@@ -142,7 +142,7 @@ Ashijeet Acharya <ashijeet.acharya@6wind.com>
Ashish Gupta <ashishg@marvell.com> <ashish.gupta@marvell.com> <ashish.gupta@caviumnetworks.com>
Ashish Jain <ashish.jain@nxp.com>
Ashish Paul <apaul@juniper.net>
-Ashish Sadanandan <ashish.sadanandan@gmail.com>
+Ashish Sadanandan <ashish.sadanandan@gmail.com> <quic_asadanan@quicinc.com>
Ashish Shah <ashish.n.shah@intel.com>
Ashwin Sekhar T K <asekhar@marvell.com> <ashwin.sekhar@caviumnetworks.com>
Asim Jamshed <asim.jamshed@gmail.com>
@@ -106,6 +106,27 @@ Headers should be protected against multiple inclusion with the usual:
#endif /* _FILE_H_ */
+Language Linkage
+~~~~~~~~~~~~~~~~
+
+Public headers should enclose all function and variable declarations and definitions in an ``extern "C"`` block to facilitate interoperability with C++.
+
+.. code-block:: c
+
+ #ifndef _FILE_H_
+ #define _FILE_H_
+
+ #ifdef __cplusplus
+ extern "C" {
+ #endif
+
+ /* Code */
+
+ #ifdef __cplusplus
+ }
+ #endif
+
+ #endif /* _FILE_H_ */
Macros
~~~~~~
@@ -15,6 +15,10 @@
#include <stdint.h>
+#ifdef __cplusplus
+extern "C" {
+#endif
+
#ifndef RTE_TOOLCHAIN_MSVC
/* Unsigned vector types */
@@ -226,4 +230,8 @@ uint16_t rte_vect_get_max_simd_bitwidth(void);
*/
int rte_vect_set_max_simd_bitwidth(uint16_t bitwidth);
+#ifdef __cplusplus
+}
+#endif
+
#endif /* _RTE_VECT_H_ */