Re: [dpdk-stable] [PATCH] eal/headers: explicitly cast void * to type *

[Bruce Richardson <bruce.richardson@intel.com>]

On Thu, Jan 14, 2021 at 10:05:57AM +0300, Dmitry Kozlyuk wrote:
> On Wed, 13 Jan 2021 21:45:49 -0800, Tyler Retzlaff wrote:
> > On Wed, Jan 13, 2021 at 08:52:55PM +0300, Dmitry Kozlyuk wrote:
> > > On Mon, 11 Jan 2021 17:18:45 -0800, Tyler Retzlaff wrote:  
> > > > Explicitly cast void * to type * so that eal headers may be
> > > > compiled as C or C++.  
> > > 
> > > Topic should probably be "eal/windows".  
> > 
> > i'll submit a new rev that changes this, it's not really windows
> > specific but i guess windows is the only people crazy enough to use
> > c++.
> 
> 1. Topic usually describe area of changes, rte_os.h is in windows
> directory.
> 
> 2. It's a perfectly valid concern that public headers must be usable from
> C++.
> 
> 
> > > > diff --git a/lib/librte_ethdev/rte_ethdev_pci.h
> > > > b/lib/librte_ethdev/rte_ethdev_pci.h index bf715896a..c20be29b1
> > > > 100644 --- a/lib/librte_ethdev/rte_ethdev_pci.h +++
> > > > b/lib/librte_ethdev/rte_ethdev_pci.h @@ -47,7 +47,7 @@
> > > > 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; +	struct rte_pci_device *pci_dev =
> > > >  (struct rte_pci_device *)bus_device;
> > > >  
> > > >  	if (!pci_dev) return -ENODEV;  
> > > 
> > > This is a private header, it's never exposed---why the change is
> > > needed (not that I have a strong opinion, though)?  
> > 
> > interesting, i'll look into why/how it is being included and confirm. i
> > suppose the question in the back of my mind is if it is private then
> > why is the header being installed at all?
> 
> + Bruce
> 
> If it's a public header then maybe it's missing a @file?
>
My 2c on this in general...
 
The use of public vs private headers is not always clear, sadly, in DPDK,
for historical reasons.  With the make builds, libraries picked up headers
from other libraries via the "include" folder for all of DPDK, meaning that
if a particular header was internal only but used by multiple other libs,
it was placed in "include" for simplicity, rather than having each library
using it having to have separate "-I/path/to/header" cflags specified.
With the switch to meson, this common folder use is no longer be the case,
but because of the old way of doing things it may be that in the transition
some private headers were inadvertently kept as public (and possibly vice
versa, though that is more likely to be spotted by now).

/Bruce