Re: [dpdk-dev] [PATCH] ethdev: missing typecast from void in eth_dev_pci_specific_init

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/12/2019 6:15 PM, Ananyev, Konstantin wrote:
> 
> 
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Friday, April 12, 2019 6:09 PM
>> To: Stephen Hemminger <stephen@networkplumber.org>; Richardson, Bruce <bruce.richardson@intel.com>
>> Cc: David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net; arybchenko@solarflare.com; dev@dpdk.org;
>> radhika.chirra@ibm.com; stable@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.richardson@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: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: David Christensen <drc@linux.vnet.ibm.com>
>>>>> Tested-by: Radhika Chirra <radhika.chirra@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.