DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
Cc: Tyler Retzlaff <roretzla@linux.microsoft.com>,
	dev@dpdk.org, navasile@linux.microsoft.com, stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] eal/headers: explicitly cast void * to type *
Date: Thu, 14 Jan 2021 10:55:54 +0000
Message-ID: <20210114105554.GA1959@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20210114100557.3ec44961@sovereign>

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

  reply	other threads:[~2021-01-14 10:56 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-12  1:18 Tyler Retzlaff
2021-01-13 17:52 ` Dmitry Kozlyuk
2021-01-14  5:45   ` Tyler Retzlaff
2021-01-14  7:05     ` Dmitry Kozlyuk
2021-01-14 10:55       ` Bruce Richardson [this message]
2021-01-14 18:27         ` Dmitry Kozlyuk
2021-01-14 18:49           ` Thomas Monjalon
2021-01-15 19:21         ` Tyler Retzlaff
2021-01-17 17:13           ` Thomas Monjalon
2021-01-15 19:38 ` [dpdk-dev] [PATCH v2] eal/windows: " Tyler Retzlaff
2021-01-17 17:19   ` Thomas Monjalon
2021-01-17 18:10   ` Stephen Hemminger
2021-01-17 19:51     ` Dmitry Kozlyuk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210114105554.GA1959@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=dmitry.kozliuk@gmail.com \
    --cc=navasile@linux.microsoft.com \
    --cc=roretzla@linux.microsoft.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git