| Message ID | 20241126112212.64524-2-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/boards: Remove legacy MachineClass::pci_allow_0_address flag | expand |
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? > @@ -32,7 +32,7 @@ enum PCIBusFlags { > > struct PCIBus { > BusState qbus; > - enum PCIBusFlags flags; > + unsigned flags; > const PCIIOMMUOps *iommu_ops; > void *iommu_opaque; > uint8_t devfn_min;
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
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 */ @@ -32,7 +32,7 @@ enum PCIBusFlags { struct PCIBus { BusState qbus; - enum PCIBusFlags flags; + unsigned flags; const PCIIOMMUOps *iommu_ops; void *iommu_opaque; uint8_t devfn_min;
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(-)