diff mbox series

[v2] ethdev: missing typecast causes C++ build error

Message ID 1554932214-96522-1-git-send-email-drc@linux.vnet.ibm.com (mailing list archive)
State Rejected, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] ethdev: missing typecast causes C++ build error |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

David Christensen April 10, 2019, 9:36 p.m. UTC 
  The function eth_dev_pci_specific_init is missing a typecast to
(struct rte_pci_device *) for the input argument bus_device.
This causes build issues in the GNU C++ compiler.

[CXX]       g++ -o utils/pcap_handle.o -c utils/pcap_handle.cc -g3·
-ggdb3 -mcpu=native -mtune=native -isystem·
/home/rchirra/gen/io/ppc64include -std=gnu++11 -flax-vector-conversions·
-Werror -isystem /home/rchirra/gen/io

In file included from drivers/pmd.cc:7:0:
/home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
/home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
[-fpermissive]
  struct rte_pci_device *pci_dev = bus_device;
                                   ^~~~~~~~~~
make[1]: *** [drivers/pmd.o] Error 1

$ g++ --version
g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0

Cc: stable@dpdk.org

Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
Tested-by: Radhika Chirra <radhika.chirra@ibm.com>
---
v2:
* Mmodified subject line to indicate a C++ compiler issue
* Added error output from the compiler

 lib/librte_ethdev/rte_ethdev_pci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ferruh Yigit April 16, 2019, 4:31 p.m. UTC | #1 
On 4/10/2019 10:36 PM, David Christensen wrote:
> The function eth_dev_pci_specific_init is missing a typecast to
> (struct rte_pci_device *) for the input argument bus_device.
> This causes build issues in the GNU C++ compiler.
> 
> [CXX]       g++ -o utils/pcap_handle.o -c utils/pcap_handle.cc -g3·
> -ggdb3 -mcpu=native -mtune=native -isystem·
> /home/rchirra/gen/io/ppc64include -std=gnu++11 -flax-vector-conversions·
> -Werror -isystem /home/rchirra/gen/io
> 
> In file included from drivers/pmd.cc:7:0:
> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
> In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
> error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
> [-fpermissive]
>   struct rte_pci_device *pci_dev = bus_device;
>                                    ^~~~~~~~~~
> make[1]: *** [drivers/pmd.o] Error 1
> 
> $ g++ --version
> g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0

This problem is while building a driver with c++ compiler, but since the
solution is a simple casting, which doesn't have any side affect, I am for
getting the patch, any objection?

> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
> Tested-by: Radhika Chirra <radhika.chirra@ibm.com>
> ---
> v2:
> * Mmodified subject line to indicate a C++ compiler issue
> * Added error output from the compiler
> 
>  lib/librte_ethdev/rte_ethdev_pci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev_pci.h b/lib/librte_ethdev/rte_ethdev_pci.h
> index 23257e9..a325311 100644
> --- a/lib/librte_ethdev/rte_ethdev_pci.h
> +++ b/lib/librte_ethdev/rte_ethdev_pci.h
> @@ -72,7 +72,7 @@
>  
>  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;
> +	struct rte_pci_device *pci_dev = (struct rte_pci_device *)bus_device;
>  
>  	if (!pci_dev)
>  		return -ENODEV;
>
Andrew Rybchenko April 16, 2019, 4:39 p.m. UTC | #2 
On 4/16/19 7:31 PM, Ferruh Yigit wrote:
> On 4/10/2019 10:36 PM, David Christensen wrote:
>> The function eth_dev_pci_specific_init is missing a typecast to
>> (struct rte_pci_device *) for the input argument bus_device.
>> This causes build issues in the GNU C++ compiler.
>>
>> [CXX]       g++ -o utils/pcap_handle.o -c utils/pcap_handle.cc -g3·
>> -ggdb3 -mcpu=native -mtune=native -isystem·
>> /home/rchirra/gen/io/ppc64include -std=gnu++11 -flax-vector-conversions·
>> -Werror -isystem /home/rchirra/gen/io
>>
>> In file included from drivers/pmd.cc:7:0:
>> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
>> In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
>> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
>> error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
>> [-fpermissive]
>>    struct rte_pci_device *pci_dev = bus_device;
>>                                     ^~~~~~~~~~
>> make[1]: *** [drivers/pmd.o] Error 1
>>
>> $ g++ --version
>> g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0
> This problem is while building a driver with c++ compiler, but since the
> solution is a simple casting, which doesn't have any side affect, I am for
> getting the patch, any objection?

It should be a decision that we support PMDs in C++ (if I understand
the usage correctly) and it will require all headers to be compatible and
ideally it should be checked by build automation (otherwise we will break
and fix it pretty often).
Ananyev, Konstantin April 16, 2019, 4:46 p.m. UTC | #3 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
> Sent: Tuesday, April 16, 2019 5:40 PM
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net
> Cc: dev@dpdk.org; radhika.chirra@ibm.com; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2] ethdev: missing typecast causes C++ build error
> 
> On 4/16/19 7:31 PM, Ferruh Yigit wrote:
> > On 4/10/2019 10:36 PM, David Christensen wrote:
> >> The function eth_dev_pci_specific_init is missing a typecast to
> >> (struct rte_pci_device *) for the input argument bus_device.
> >> This causes build issues in the GNU C++ compiler.
> >>
> >> [CXX]       g++ -o utils/pcap_handle.o -c utils/pcap_handle.cc -g3·
> >> -ggdb3 -mcpu=native -mtune=native -isystem·
> >> /home/rchirra/gen/io/ppc64include -std=gnu++11 -flax-vector-conversions·
> >> -Werror -isystem /home/rchirra/gen/io
> >>
> >> In file included from drivers/pmd.cc:7:0:
> >> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
> >> In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
> >> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
> >> error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
> >> [-fpermissive]
> >>    struct rte_pci_device *pci_dev = bus_device;
> >>                                     ^~~~~~~~~~
> >> make[1]: *** [drivers/pmd.o] Error 1
> >>
> >> $ g++ --version
> >> g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0
> > This problem is while building a driver with c++ compiler, but since the
> > solution is a simple casting, which doesn't have any side affect, I am for
> > getting the patch, any objection?
> 
> It should be a decision that we support PMDs in C++ (if I understand
> the usage correctly) and it will require all headers to be compatible and
> ideally it should be checked by build automation (otherwise we will break
> and fix it pretty often).

Or as alternative, we probably can claim that PMDs in C++ are not supported,
and if people like to do that - they have to deal with it on their own
(create a C wrapper file, or so).
Konstantin
Stephen Hemminger April 16, 2019, 9:19 p.m. UTC | #4 
On Tue, 16 Apr 2019 16:46:14 +0000
"Ananyev, Konstantin" <konstantin.ananyev@intel.com> wrote:

> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Andrew Rybchenko
> > Sent: Tuesday, April 16, 2019 5:40 PM
> > To: Yigit, Ferruh <ferruh.yigit@intel.com>; David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net
> > Cc: dev@dpdk.org; radhika.chirra@ibm.com; stable@dpdk.org
> > Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v2] ethdev: missing typecast causes C++ build error
> > 
> > On 4/16/19 7:31 PM, Ferruh Yigit wrote:  
> > > On 4/10/2019 10:36 PM, David Christensen wrote:  
> > >> The function eth_dev_pci_specific_init is missing a typecast to
> > >> (struct rte_pci_device *) for the input argument bus_device.
> > >> This causes build issues in the GNU C++ compiler.
> > >>
> > >> [CXX]       g++ -o utils/pcap_handle.o -c utils/pcap_handle.cc -g3·
> > >> -ggdb3 -mcpu=native -mtune=native -isystem·
> > >> /home/rchirra/gen/io/ppc64include -std=gnu++11 -flax-vector-conversions·
> > >> -Werror -isystem /home/rchirra/gen/io
> > >>
> > >> In file included from drivers/pmd.cc:7:0:
> > >> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
> > >> In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
> > >> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
> > >> error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
> > >> [-fpermissive]
> > >>    struct rte_pci_device *pci_dev = bus_device;
> > >>                                     ^~~~~~~~~~
> > >> make[1]: *** [drivers/pmd.o] Error 1
> > >>
> > >> $ g++ --version
> > >> g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0  
> > > This problem is while building a driver with c++ compiler, but since the
> > > solution is a simple casting, which doesn't have any side affect, I am for
> > > getting the patch, any objection?  
> > 
> > It should be a decision that we support PMDs in C++ (if I understand
> > the usage correctly) and it will require all headers to be compatible and
> > ideally it should be checked by build automation (otherwise we will break
> > and fix it pretty often).  
> 
> Or as alternative, we probably can claim that PMDs in C++ are not supported,
> and if people like to do that - they have to deal with it on their own
> (create a C wrapper file, or so).
> Konstantin
> 


+1 no drivers or other parts of EAL in C++
David Christensen April 16, 2019, 9:50 p.m. UTC | #5 
>>>>> In file included from drivers/pmd.cc:7:0:
>>>>> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:
>>>>> In function ‘int eth_dev_pci_specific_init(rte_eth_dev*, void*)’:
>>>>> /home/rchirra/gen/io/deps/dpdk/build/include/rte_ethdev_pci.h:75:35:·
>>>>> error: invalid conversion from ‘void*’ to ‘rte_pci_device*’
>>>>> [-fpermissive]
>>>>>     struct rte_pci_device *pci_dev = bus_device;
>>>>>                                      ^~~~~~~~~~
>>>>> make[1]: *** [drivers/pmd.o] Error 1
>>>>>
>>>>> $ g++ --version
>>>>> g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0
>>>> This problem is while building a driver with c++ compiler, but since the
>>>> solution is a simple casting, which doesn't have any side affect, I am for
>>>> getting the patch, any objection?
>>>
>>> It should be a decision that we support PMDs in C++ (if I understand
>>> the usage correctly) and it will require all headers to be compatible and
>>> ideally it should be checked by build automation (otherwise we will break
>>> and fix it pretty often).
>>
>> Or as alternative, we probably can claim that PMDs in C++ are not supported,
>> and if people like to do that - they have to deal with it on their own
>> (create a C wrapper file, or so).
>> Konstantin
>>
> +1 no drivers or other parts of EAL in C++
> 
I've learned more about the intended usage in this case and it turns out 
not to be a PMD at all, it's an application that's reaching into the EAL 
in what I consider an inappropriate way.  As a result I'm withdrawing 
the patch request and I'll work with the developers to find an alternate 
solution to their problem.

Dave
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev_pci.h b/lib/librte_ethdev/rte_ethdev_pci.h
index 23257e9..a325311 100644
--- a/lib/librte_ethdev/rte_ethdev_pci.h
+++ b/lib/librte_ethdev/rte_ethdev_pci.h
@@ -72,7 +72,7 @@ 
 
 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;
+	struct rte_pci_device *pci_dev = (struct rte_pci_device *)bus_device;
 
 	if (!pci_dev)
 		return -ENODEV;