Re: [PATCH-for-10.0 v2 01/13] hw/pci: Do not declare PCIBus::flags mask as enum

[Thomas Huth]

On 27/11/2024 10.37, Philippe Mathieu-Daudé wrote:
> On 26/11/24 12:22, Philippe Mathieu-Daudé wrote:
> > We use PCIBus::flags to mask various flags. It is not
> > an enum, and doing so confuses static analyzers. Rename
> > the enum as singular. Use a generic unsigned type for
> > the mask.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   include/hw/pci/pci_bus.h | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> > index 22613125462..6ecfe2e06d5 100644
> > --- a/include/hw/pci/pci_bus.h
> > +++ b/include/hw/pci/pci_bus.h
> > @@ -19,7 +19,7 @@ struct PCIBusClass {
> >       uint16_t (*numa_node)(PCIBus *bus);
> >   };
> > -enum PCIBusFlags {
> > +enum PCIBusFlag {
> >       /* This bus is the root of a PCI domain */
> >       PCI_BUS_IS_ROOT                                         = 0x0001,
> >       /* PCIe extended configuration space is accessible on this bus */
>
> (more diff context:)
>
>          PCI_BUS_EXTENDED_CONFIG_SPACE                           = 0x0002,
>         /* This is a CXL Type BUS */
>         PCI_BUS_CXL                                          = 0x0004,
>
> Enum would be the [0, 1, 2] bits. Since we define bitmask and use
> bitmask arguments in the code, shouldn't we simply replace that
> enum by #define?

Agreed, this rather sounds like #defines than an enum to me, too.

 Thomas