Re: [PATCH 4/5] include/hw/pci: Split pci_device.h off pci.h

[Markus Armbruster <armbru@redhat.com>]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Fri, Dec 09, 2022 at 02:48:01PM +0100, Markus Armbruster wrote:
>> PCIDeviceClass and PCIDevice are defined in pci.h.  Many users of the
>> header don't actually need them.  Similar structs live in their own
>> headers: PCIBusClass and PCIBus in pci_bus.h, PCIBridge in
>> pci_bridge.h, PCIHostBridgeClass and PCIHostState in pci_host.h,
>> PCIExpressHost in pcie_host.h, and PCIERootPortClass, PCIEPort, and
>> PCIESlot in pcie_port.h.
>> 
>> Move PCIDeviceClass and PCIDeviceClass to new pci_device.h, along with
>> the code that needs them.  Adjust include directives.
>> 
>> This also enables the next commit.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>
>
> checkpatch is unhappy:
>
> ./scripts/checkpatch.pl /tmp/patch 
> WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
> #707: 
> new file mode 100644

We're good:

    $ scripts/get_maintainer.pl -f include/hw/pci/pci_device.h 
    "Michael S. Tsirkin" <mst@redhat.com> (supporter:PCI)
    Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PCI)
    qemu-devel@nongnu.org (open list:All patches CC here)

But checkpatch is too simple-minded to see that.

> ERROR: spaces required around that '*' (ctx:WxV)
> #997: FILE: include/hw/pci/pci_device.h:286:
> +                                               uint##_bits##_t *val, \
>                                                                 ^

False positive.

  #define PCI_DMA_DEFINE_LDST(_l, _s, _bits) \
      static inline MemTxResult ld##_l##_pci_dma(PCIDevice *dev, \
                                                 dma_addr_t addr, \
--->                                             uint##_bits##_t *val, \
                                                 MemTxAttrs attrs) \
      { \
          return ld##_l##_dma(pci_get_address_space(dev), addr, val, attrs); \
      } \
      static inline MemTxResult st##_s##_pci_dma(PCIDevice *dev, \
                                                 dma_addr_t addr, \
                                                 uint##_bits##_t val, \
                                                 MemTxAttrs attrs) \
      { \
          return st##_s##_dma(pci_get_address_space(dev), addr, val, attrs); \
      }

The part checkpatch objects to is actually a parameter declaration of
the form

    TYPE *NAME

The use of spaces is fine.  Not fine would be

    TYPE * NAME

Having a macro expand into a function definition confuses checkpatch.

> It's right - we need a MAINTAINERS entry.
> Not sure how to fix the error - any idea?

There is nothing to fix :)