diff mbox series

[6/7] vhost: remove non-C++ compatible includes

Message ID 20220204174209.440207-7-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Verify C++ compatibility of public headers |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Feb. 4, 2022, 5:42 p.m. UTC 
  Some of the linux header includes are explicitly noted as being
incompatible with C++. However, these headers can included by C files
directly, or by internal headers, to avoid polluting the public DPDK
headers with non-C++ safe includes.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 2 ++
 drivers/vdpa/ifc/ifcvf_vdpa.c     | 2 ++
 drivers/vdpa/mlx5/mlx5_vdpa.h     | 1 +
 lib/vhost/rte_vhost.h             | 4 ----
 4 files changed, 5 insertions(+), 4 deletions(-)

--
2.32.0

Comments

Bruce Richardson Feb. 4, 2022, 6:18 p.m. UTC | #1 
On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> Some of the linux header includes are explicitly noted as being
> incompatible with C++. However, these headers can included by C files
> directly, or by internal headers, to avoid polluting the public DPDK
> headers with non-C++ safe includes.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

CI is reporting build issues with this patch on examples, something I'm not
surprised to see. I will wait for maintainer feedback on best approach
before respinning patchset.

/Bruce
Xiao Wang Feb. 9, 2022, 9:10 a.m. UTC | #2 
Hi Bruce,

> -----Original Message-----
> From: Richardson, Bruce <bruce.richardson@intel.com>
> Sent: Saturday, February 5, 2022 2:19 AM
> To: dev@dpdk.org
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia, Chenbo
> <chenbo.xia@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>; Matan
> Azrad <matan@nvidia.com>; Viacheslav Ovsiienko
> <viacheslavo@nvidia.com>
> Subject: Re: [PATCH 6/7] vhost: remove non-C++ compatible includes
> 
> On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> > Some of the linux header includes are explicitly noted as being
> > incompatible with C++. However, these headers can included by C files
> > directly, or by internal headers, to avoid polluting the public DPDK
> > headers with non-C++ safe includes.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> 
> CI is reporting build issues with this patch on examples, something I'm not
> surprised to see. I will wait for maintainer feedback on best approach
> before respinning patchset.
> 
> /Bruce

Could we move these c++ incompatible linux headers into
#ifndef __cplusplus
...
#endif.
Then we just need to change rte_vhost.h file, and don't break build for the drivers and samples.

BRs,
Xiao
Bruce Richardson Feb. 9, 2022, 9:21 a.m. UTC | #3 
On Wed, Feb 09, 2022 at 09:10:36AM +0000, Wang, Xiao W wrote:
> Hi Bruce,
> 
> > -----Original Message----- From: Richardson, Bruce
> > <bruce.richardson@intel.com> Sent: Saturday, February 5, 2022 2:19 AM
> > To: dev@dpdk.org Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; Xia,
> > Chenbo <chenbo.xia@intel.com>; Wang, Xiao W <xiao.w.wang@intel.com>;
> > Matan Azrad <matan@nvidia.com>; Viacheslav Ovsiienko
> > <viacheslavo@nvidia.com> Subject: Re: [PATCH 6/7] vhost: remove non-C++
> > compatible includes
> >
> > On Fri, Feb 04, 2022 at 05:42:08PM +0000, Bruce Richardson wrote:
> > > Some of the linux header includes are explicitly noted as being
> > > incompatible with C++. However, these headers can included by C files
> > > directly, or by internal headers, to avoid polluting the public DPDK
> > > headers with non-C++ safe includes.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com> ---
> >
> > CI is reporting build issues with this patch on examples, something I'm
> > not surprised to see. I will wait for maintainer feedback on best
> > approach before respinning patchset.
> >
> > /Bruce
> 
> Could we move these c++ incompatible linux headers into #ifndef
> __cplusplus ...  #endif.  Then we just need to change rte_vhost.h file,
> and don't break build for the drivers and samples.
>
I was thinking that something similar would work, but it's not the most
elegant, and strikes me as a bit of a hack. If it's generally acceptable,
though, I'm happy enough to respin the patch with that fix.

/Bruce
diff mbox series

Patch

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 070f0e6dfd..cd2afe3100 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -16,6 +16,8 @@ 
 #include <rte_kvargs.h>
 #include <rte_vhost.h>
 #include <rte_spinlock.h>
+#include <linux/vhost.h>
+#include <linux/virtio_net.h>

 #include "rte_eth_vhost.h"

diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
index 3853c4cf7e..cbd190ea8d 100644
--- a/drivers/vdpa/ifc/ifcvf_vdpa.c
+++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
@@ -24,6 +24,8 @@ 
 #include <rte_kvargs.h>
 #include <rte_devargs.h>

+#include <linux/vhost_types.h>
+
 #include "base/ifcvf.h"

 RTE_LOG_REGISTER(ifcvf_vdpa_logtype, pmd.vdpa.ifcvf, NOTICE);
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h
index 22617924ea..a7fa4356dd 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.h
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
@@ -6,6 +6,7 @@ 
 #define RTE_PMD_MLX5_VDPA_H_

 #include <linux/virtio_net.h>
+#include <linux/vhost_types.h>
 #include <sys/queue.h>

 #ifdef PEDANTIC
diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index b454c05868..0a21177199 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -21,10 +21,6 @@ 
 extern "C" {
 #endif

-/* These are not C++-aware. */
-#include <linux/vhost.h>
-#include <linux/virtio_ring.h>
-#include <linux/virtio_net.h>

 #define RTE_VHOST_USER_CLIENT		(1ULL << 0)
 #define RTE_VHOST_USER_NO_RECONNECT	(1ULL << 1)