All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Marcel Apfelbaum <marcel@redhat.com>
Subject: Re: How to best make include/hw/pci/pcie_sriov.h self-contained
Date: Wed, 7 Dec 2022 05:18:41 -0500	[thread overview]
Message-ID: <20221207020305-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <87bkofivbm.fsf@pond.sub.org>

On Wed, Dec 07, 2022 at 07:25:49AM +0100, Markus Armbruster wrote:
> pcie_sriov.h needs PCI_NUM_REGIONS from pci.h, but doesn't include it.
> pci.h must be included before pcie_sriov.h or else compile fails.
> 
> Adding #include "pci/pci.h" to pcie_sriov would be wrong, because it
> would close an inclusion loop: pci.h includes pcie.h (for
> PCIExpressDevice) includes pcie_sriov.h (for PCIESriovPF) includes pci.h
> (for PCI_NUM_REGIONS).
> 
> The obvious solution is to move PCI_NUM_REGIONS pci.h somewhere
> pcie_sriov.h can include without creating a loop.
> 
> We already have a few headers that don't include anything: pci_ids.h,
> pci_regs.h (includes include/standard-headers/linux/pci_regs.h, which
> doesn't count), pcie_regs.h.  Moving PCI_NUM_REGIONS to one of these
> would work, but it doesn't feel right.
> 
> We could create a new one, say pci_defs.h.  Just for PCI_NUM_REGIONS
> feels silly.  So, what else should move there?

I'm ok with pci_defs.h
However, I note that most headers including pci.h don't really
need it. Consider include/hw/virtio/virtio-iommu.h all it needs is
PCIBus typedef this is available from qemu/typedefs.h
So if you are poking at this, want to clean that area up generally?

> Any other ideas?
> 
> In case you wonder why I bother you with this...
> 
> Back in 2016, we discussed[1] rules for headers, and these were
> generally liked:
> 
> 1. Have a carefully curated header that's included everywhere first.  We
>    got that already thanks to Peter: osdep.h.
> 
> 2. Headers should normally include everything they need beyond osdep.h.
>    If exceptions are needed for some reason, they must be documented in
>    the header.  If all that's needed from a header is typedefs, put
>    those into qemu/typedefs.h instead of including the header.
> 
> 3. Cyclic inclusion is forbidden.
> 
> I'm working on patches to get include/ closer to obeying 2.
> 
> [1] Message-ID: <87h9g8j57d.fsf@blackfin.pond.sub.org>
>     https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html



  parent reply	other threads:[~2022-12-07 10:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-07  6:25 How to best make include/hw/pci/pcie_sriov.h self-contained Markus Armbruster
2022-12-07  9:02 ` Philippe Mathieu-Daudé
2022-12-07 10:29   ` Michael S. Tsirkin
2022-12-07 10:18 ` Michael S. Tsirkin [this message]
2022-12-09 13:09   ` Markus Armbruster

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=20221207020305-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=armbru@redhat.com \
    --cc=marcel@redhat.com \
    --cc=qemu-devel@nongnu.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.