On Fri, 12 Apr 2019 18:29:46 +0100 Ferruh Yigit <ferruh.yi...@intel.com> wrote:
> On 4/12/2019 6:25 PM, Ferruh Yigit wrote: > > On 4/12/2019 6:15 PM, Ananyev, Konstantin wrote: > >> > >> > >>> -----Original Message----- > >>> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ferruh Yigit > >>> Sent: Friday, April 12, 2019 6:09 PM > >>> To: Stephen Hemminger <step...@networkplumber.org>; Richardson, Bruce > >>> <bruce.richard...@intel.com> > >>> Cc: David Christensen <d...@linux.vnet.ibm.com>; tho...@monjalon.net; > >>> arybche...@solarflare.com; dev@dpdk.org; > >>> radhika.chi...@ibm.com; sta...@dpdk.org > >>> Subject: Re: [dpdk-dev] [PATCH] ethdev: missing typecast from void in > >>> eth_dev_pci_specific_init > >>> > >>> On 4/11/2019 12:08 AM, Stephen Hemminger wrote: > >>>> On Wed, 10 Apr 2019 22:00:18 +0100 > >>>> Bruce Richardson <bruce.richard...@intel.com> wrote: > >>>> > >>>>> On Wed, Apr 10, 2019 at 03:16:16PM -0500, 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. > >>>>>> > >>>>>> Cc: sta...@dpdk.org > >>>>>> > >>>>>> Signed-off-by: David Christensen <d...@linux.vnet.ibm.com> > >>>>>> Tested-by: Radhika Chirra <radhika.chi...@ibm.com> > >>>>>> --- > >>>>>> 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; > >>>>>> > >>>>> > >>>>> Is this needed for building some C++ apps that are including the header > >>>>> file (directly, or indirectly), because for pure C, "void *" types > >>>>> should > >>>>> be assignable to any other pointer type without casting? > >>>>> > >>>>> /Bruce > >>>> > >>>> Another example of Why the Hell is this inline? > >>>> > >>> > >>> It has been done inline intentionally at the time as far as remember, this > >>> header is for drivers not for applications, it has helper functions. > >>> > >>> The common code from drivers related to the bus put into header files, so > >>> the > >>> code itself belongs to drivers not ethdev and reduces duplicates in them. > >>> > >> > >> Ok that's the common code used by the drivers... > >> But why it still can't be in .c file? > > > > When it is in .c file, it will be either in ethdev library, single location > > in > > .c file and binary file, but location is not exactly right, because code > > belongs > > to drivers. > > Or code should be in .c files of each drivers, this will be code > > duplication. > > > > Having in .h file makes code in single place, but when compiled code will > > be in > > each driver object file/ library. > > > > Of course it works when put into a .c file in ehtdev, but bus (pci and vdev) > > related code are not belongs to ethdev library and I believe shouldn't be > > part > > of ethdev binary. And those bus helper headers are only for drivers to > > include, > > so having inline shouldn't be a problem at all because there is not > > stability > > concern in that interface. > > > > btw, if you put those into .c file in ethdev, you will be creating a > dependency > from ethdev to bus code, to all available buses which will make impossible to > disable any bus type if you use ethdev. The problem I see is rte_ethdev_pci.h, it should be headers only and then put code rte_ethdev_pci.c
