[PATCH 0/3] extend C++ compatibility checks

[PATCH 0/3] extend C++ compatibility checks

[Bruce Richardson]

This set expands upon existing checks for C++ compatibility, adding in
checks for both the SDK headers and a basic check for the presence of
'extern "C"' guards in each file.

Depends-on: series-21685 ("add missing C++ guards")


Bruce Richardson (3):
  ethdev: fix missing cast for C++ compatibility
  buildtools/chkincs: check sdk headers for C++ compatibility
  buildtools/chkincs: add checks for missing C++ guards

 buildtools/chkincs/meson.build | 16 +++++++++++++++-
 lib/ethdev/ethdev_pci.h        |  5 +++--
 lib/meson.build                |  1 +
 3 files changed, 19 insertions(+), 3 deletions(-)

--
2.32.0

[PATCH 1/3] ethdev: fix missing cast for C++ compatibility

[Bruce Richardson]

C++ does not allow implicit conversion to/from void*, so we need an
explicit cast to allow the driver sdk header to be included from C++
code.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/ethdev/ethdev_pci.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
index 71aa4b2e98..d2bc3fe5e0 100644
--- a/lib/ethdev/ethdev_pci.h
+++ b/lib/ethdev/ethdev_pci.h
@@ -46,8 +46,9 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
 }
 
 static inline int
-eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device) {
-	struct rte_pci_device *pci_dev = bus_device;
+eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device)
+{
+	struct rte_pci_device *pci_dev = (struct rte_pci_device *)bus_device;
 
 	if (!pci_dev)
 		return -ENODEV;
-- 
2.32.0

Re: [PATCH 1/3] ethdev: fix missing cast for C++ compatibility

[Ferruh Yigit]

On 2/15/2022 5:30 PM, Bruce Richardson wrote:
> C++ does not allow implicit conversion to/from void*, so we need an
> explicit cast to allow the driver sdk header to be included from C++
> code.
> 

I remember patches removing explicit "void *" cast, in the past,

to document, is the rule as following:
- public and sdk headers should support c++, hence these files
   must have explicit "void *" cast
- .c files should NOT have explicit "void *" cast

> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>   lib/ethdev/ethdev_pci.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/ethdev/ethdev_pci.h b/lib/ethdev/ethdev_pci.h
> index 71aa4b2e98..d2bc3fe5e0 100644
> --- a/lib/ethdev/ethdev_pci.h
> +++ b/lib/ethdev/ethdev_pci.h
> @@ -46,8 +46,9 @@ rte_eth_copy_pci_info(struct rte_eth_dev *eth_dev,
>   }
>   
>   static inline int
> -eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device) {
> -	struct rte_pci_device *pci_dev = bus_device;
> +eth_dev_pci_specific_init(struct rte_eth_dev *eth_dev, void *bus_device)
> +{
> +	struct rte_pci_device *pci_dev = (struct rte_pci_device *)bus_device;
>   
>   	if (!pci_dev)
>   		return -ENODEV;

Re: [PATCH 1/3] ethdev: fix missing cast for C++ compatibility

[Bruce Richardson]

On Wed, Feb 16, 2022 at 10:32:43AM +0000, Ferruh Yigit wrote:
> On 2/15/2022 5:30 PM, Bruce Richardson wrote:
> > C++ does not allow implicit conversion to/from void*, so we need an
> > explicit cast to allow the driver sdk header to be included from C++
> > code.
> > 
> 
> I remember patches removing explicit "void *" cast, in the past,
> 
> to document, is the rule as following:
> - public and sdk headers should support c++, hence these files
>   must have explicit "void *" cast
> - .c files should NOT have explicit "void *" cast
> 
That would be my understanding yes. It's annoying they conflict, so I would
suggest softening the second rule to allow casts to be present. Thankfully
with chkincs support we should be able to detect violations of rule #1,
which is the more important of the two.

/Bruce

[PATCH 2/3] buildtools/chkincs: check sdk headers for C++ compatibility

[Bruce Richardson]

With a one-line change to the lib meson.build file we can add the sdk
headers to the list of files to be checked using the chkincs binary.
Unfortunately, many of those sdk header depend upon headers in the PCI
and vdev bus drivers, so we need to update chkincs build to ensure those
dependencies are added. We also need to allow internal APIs to be
present in these SDK headers.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/meson.build | 6 +++++-
 lib/meson.build                | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 790f700619..9442235200 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -13,11 +13,15 @@ gen_c_files = generator(gen_c_file_for_header,

 cflags = machine_args
 cflags += '-DALLOW_EXPERIMENTAL_API'
+cflags += '-DALLOW_INTERNAL_API'

 sources = files('main.c')
 sources += gen_c_files.process(dpdk_chkinc_headers)

-deps = []
+# some driver sdk headers depend on these two buses, which are mandatory in build
+# so we always include them in deps list
+deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
+# add the rest of the libs to the dependencies
 foreach l:enabled_libs
     deps += get_variable('shared_rte_' + l)
 endforeach
diff --git a/lib/meson.build b/lib/meson.build
index 8e5acd7819..24adbe44c9 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -171,6 +171,7 @@ foreach l:libraries
         install_headers(driver_sdk_headers)
     endif
     dpdk_chkinc_headers += headers
+    dpdk_chkinc_headers += driver_sdk_headers

     libname = 'rte_' + name
     includes += include_directories(l)
--
2.32.0

[PATCH 3/3] buildtools/chkincs: add checks for missing C++ guards

[Bruce Richardson]

Simply compiling a C header with a C++ compiler is not enough to flag
missing 'extern "C"' guards. To catch missing guards, we can just use a
simple grep for the 'extern "C"' part, and error out if any files have a
miss.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---

Depends-on: series-21685 ("add missing C++ guards")
---
 buildtools/chkincs/meson.build | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 9442235200..77ef59fb13 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -37,6 +37,16 @@ if not add_languages('cpp', required: false)
     subdir_done()
 endif

+# check for extern C in files, since this is not detected as an error by the compiler
+grep = find_program('grep', required: false)
+if grep.found()
+    errlist = run_command([grep, '--files-without-match', '^extern "C"', dpdk_chkinc_headers],
+            check: false, capture: true).stdout().split()
+    if errlist != []
+        error('Files missing C++ \'extern "C"\' guards:\n- ' + '\n- '.join(errlist))
+    endif
+endif
+
 gen_cpp_files = generator(gen_c_file_for_header,
         output: '@BASENAME@.cpp',
         arguments: ['@INPUT@', '@OUTPUT@'])
--
2.32.0

Re: [PATCH 0/3] extend C++ compatibility checks

[Tyler Retzlaff]

On Tue, Feb 15, 2022 at 05:30:26PM +0000, Bruce Richardson wrote:
> This set expands upon existing checks for C++ compatibility, adding in
> checks for both the SDK headers and a basic check for the presence of
> 'extern "C"' guards in each file.
> 
> Depends-on: series-21685 ("add missing C++ guards")

Series-Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Re: [PATCH 0/3] extend C++ compatibility checks

[Thomas Monjalon]

17/02/2022 09:06, Tyler Retzlaff:
> On Tue, Feb 15, 2022 at 05:30:26PM +0000, Bruce Richardson wrote:
> > This set expands upon existing checks for C++ compatibility, adding in
> > checks for both the SDK headers and a basic check for the presence of
> > 'extern "C"' guards in each file.
> > 
> > Depends-on: series-21685 ("add missing C++ guards")
> 
> Series-Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>

Applied, thanks.