pci_bus_is_root() currently relies on a method in the PCIBusClass.
But it's always known if a PCI bus is a root bus when we create it, so
using a dynamic method is overkill.
This replaces it with an IS_ROOT bit in a new flags field, which is set on
root buses and otherwise clear. As a bonus this removes the special
is_root logic from pci_expander_bridge, since it already creates its bus
as a root bus.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/pci-bridge/pci_expander_bridge.c | 6 ------
hw/pci/pci.c | 14 ++------------
include/hw/pci/pci.h | 12 +++++++++++-
3 files changed, 13 insertions(+), 19 deletions(-)
diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
index 5652cf06e9..11dfa9258e 100644
--- a/hw/pci-bridge/pci_expander_bridge.c
+++ b/hw/pci-bridge/pci_expander_bridge.c
@@ -65,11 +65,6 @@ static int pxb_bus_num(PCIBus *bus)
return pxb->bus_nr;
}
-static bool pxb_is_root(PCIBus *bus)
-{
- return true; /* by definition */
-}
-
static uint16_t pxb_bus_numa_node(PCIBus *bus)
{
PXBDev *pxb = convert_to_pxb(bus->parent_dev);
@@ -82,7 +77,6 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
PCIBusClass *pbc = PCI_BUS_CLASS(class);
pbc->bus_num = pxb_bus_num;
- pbc->is_root = pxb_is_root;
pbc->numa_node = pxb_bus_numa_node;
}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 6e11dc2fec..5fab7f23b3 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -126,14 +126,9 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
vmstate_unregister(NULL, &vmstate_pcibus, bus);
}
-static bool pcibus_is_root(PCIBus *bus)
-{
- return !bus->parent_dev;
-}
-
static int pcibus_num(PCIBus *bus)
{
- if (pcibus_is_root(bus)) {
+ if (pci_bus_is_root(bus)) {
return 0; /* pci host bridge */
}
return bus->parent_dev->config[PCI_SECONDARY_BUS];
@@ -156,7 +151,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
k->unrealize = pci_bus_unrealize;
k->reset = pcibus_reset;
- pbc->is_root = pcibus_is_root;
pbc->bus_num = pcibus_num;
pbc->numa_node = pcibus_numa_node;
}
@@ -385,6 +379,7 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
bus->slot_reserved_mask = 0x0;
bus->address_space_mem = address_space_mem;
bus->address_space_io = address_space_io;
+ bus->flags |= PCI_BUS_IS_ROOT;
/* host bridge */
QLIST_INIT(&bus->child);
@@ -397,11 +392,6 @@ bool pci_bus_is_express(PCIBus *bus)
return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
}
-bool pci_bus_is_root(PCIBus *bus)
-{
- return PCI_BUS_GET_CLASS(bus)->is_root(bus);
-}
-
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
const char *name,
MemoryRegion *address_space_mem,
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 77d92a3dc4..cbb3386207 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
#define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
#define TYPE_PCIE_BUS "PCIE"
+enum PCIBusFlags {
+ /* This bus is the root of a PCI domain */
+ PCI_BUS_IS_ROOT = 0x0001,
+};
+
typedef struct PCIBusClass {
/*< private >*/
BusClass parent_class;
@@ -416,6 +421,7 @@ typedef struct PCIBusClass {
struct PCIBus {
BusState qbus;
+ enum PCIBusFlags flags;
PCIIOMMUFunc iommu_fn;
void *iommu_opaque;
uint8_t devfn_min;
@@ -440,8 +446,12 @@ struct PCIBus {
Notifier machine_done;
};
+static inline bool pci_bus_is_root(PCIBus *bus)
+{
+ return !!(bus->flags & PCI_BUS_IS_ROOT);
+}
+
bool pci_bus_is_express(PCIBus *bus);
-bool pci_bus_is_root(PCIBus *bus);
void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
const char *name,
MemoryRegion *address_space_mem,
--
2.14.3
On 29/11/2017 10:46, David Gibson wrote:
> pci_bus_is_root() currently relies on a method in the PCIBusClass.
> But it's always known if a PCI bus is a root bus when we create it, so
> using a dynamic method is overkill.
>
> This replaces it with an IS_ROOT bit in a new flags field, which is set on
> root buses and otherwise clear. As a bonus this removes the special
> is_root logic from pci_expander_bridge, since it already creates its bus
> as a root bus.
>
I agree it makes the code simpler, I am not sure about
the enum. Why not make it a boolean field and when will need
more flags just convert it to an enum (or not)?
Thanks,
Marcel
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/pci-bridge/pci_expander_bridge.c | 6 ------
> hw/pci/pci.c | 14 ++------------
> include/hw/pci/pci.h | 12 +++++++++++-
> 3 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> index 5652cf06e9..11dfa9258e 100644
> --- a/hw/pci-bridge/pci_expander_bridge.c
> +++ b/hw/pci-bridge/pci_expander_bridge.c
> @@ -65,11 +65,6 @@ static int pxb_bus_num(PCIBus *bus)
> return pxb->bus_nr;
> }
>
> -static bool pxb_is_root(PCIBus *bus)
> -{
> - return true; /* by definition */
> -}
> -
> static uint16_t pxb_bus_numa_node(PCIBus *bus)
> {
> PXBDev *pxb = convert_to_pxb(bus->parent_dev);
> @@ -82,7 +77,6 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> PCIBusClass *pbc = PCI_BUS_CLASS(class);
>
> pbc->bus_num = pxb_bus_num;
> - pbc->is_root = pxb_is_root;
> pbc->numa_node = pxb_bus_numa_node;
> }
>
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6e11dc2fec..5fab7f23b3 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -126,14 +126,9 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
> vmstate_unregister(NULL, &vmstate_pcibus, bus);
> }
>
> -static bool pcibus_is_root(PCIBus *bus)
> -{
> - return !bus->parent_dev;
> -}
> -
> static int pcibus_num(PCIBus *bus)
> {
> - if (pcibus_is_root(bus)) {
> + if (pci_bus_is_root(bus)) {
> return 0; /* pci host bridge */
> }
> return bus->parent_dev->config[PCI_SECONDARY_BUS];
> @@ -156,7 +151,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
> k->unrealize = pci_bus_unrealize;
> k->reset = pcibus_reset;
>
> - pbc->is_root = pcibus_is_root;
> pbc->bus_num = pcibus_num;
> pbc->numa_node = pcibus_numa_node;
> }
> @@ -385,6 +379,7 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> bus->slot_reserved_mask = 0x0;
> bus->address_space_mem = address_space_mem;
> bus->address_space_io = address_space_io;
> + bus->flags |= PCI_BUS_IS_ROOT;
>
> /* host bridge */
> QLIST_INIT(&bus->child);
> @@ -397,11 +392,6 @@ bool pci_bus_is_express(PCIBus *bus)
> return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> }
>
> -bool pci_bus_is_root(PCIBus *bus)
> -{
> - return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> -}
> -
> void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> const char *name,
> MemoryRegion *address_space_mem,
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 77d92a3dc4..cbb3386207 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
> #define TYPE_PCIE_BUS "PCIE"
>
> +enum PCIBusFlags {
> + /* This bus is the root of a PCI domain */
> + PCI_BUS_IS_ROOT = 0x0001,
> +};
> +
> typedef struct PCIBusClass {
> /*< private >*/
> BusClass parent_class;
> @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
>
> struct PCIBus {
> BusState qbus;
> + enum PCIBusFlags flags;
> PCIIOMMUFunc iommu_fn;
> void *iommu_opaque;
> uint8_t devfn_min;
> @@ -440,8 +446,12 @@ struct PCIBus {
> Notifier machine_done;
> };
>
> +static inline bool pci_bus_is_root(PCIBus *bus)
> +{
> + return !!(bus->flags & PCI_BUS_IS_ROOT);
> +}
> +
> bool pci_bus_is_express(PCIBus *bus);
> -bool pci_bus_is_root(PCIBus *bus);
> void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> const char *name,
> MemoryRegion *address_space_mem,
>
On Wed, Nov 29, 2017 at 12:45:28PM +0200, Marcel Apfelbaum wrote:
> On 29/11/2017 10:46, David Gibson wrote:
> > pci_bus_is_root() currently relies on a method in the PCIBusClass.
> > But it's always known if a PCI bus is a root bus when we create it, so
> > using a dynamic method is overkill.
> >
> > This replaces it with an IS_ROOT bit in a new flags field, which is set on
> > root buses and otherwise clear. As a bonus this removes the special
> > is_root logic from pci_expander_bridge, since it already creates its bus
> > as a root bus.
> >
>
> I agree it makes the code simpler, I am not sure about
> the enum. Why not make it a boolean field and when will need
> more flags just convert it to an enum (or not)?
Well, I have some patches I'm working on (rather slowly) which add a
second flag.
>
> Thanks,
> Marcel
>
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> > hw/pci-bridge/pci_expander_bridge.c | 6 ------
> > hw/pci/pci.c | 14 ++------------
> > include/hw/pci/pci.h | 12 +++++++++++-
> > 3 files changed, 13 insertions(+), 19 deletions(-)
> >
> > diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
> > index 5652cf06e9..11dfa9258e 100644
> > --- a/hw/pci-bridge/pci_expander_bridge.c
> > +++ b/hw/pci-bridge/pci_expander_bridge.c
> > @@ -65,11 +65,6 @@ static int pxb_bus_num(PCIBus *bus)
> > return pxb->bus_nr;
> > }
> > -static bool pxb_is_root(PCIBus *bus)
> > -{
> > - return true; /* by definition */
> > -}
> > -
> > static uint16_t pxb_bus_numa_node(PCIBus *bus)
> > {
> > PXBDev *pxb = convert_to_pxb(bus->parent_dev);
> > @@ -82,7 +77,6 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
> > PCIBusClass *pbc = PCI_BUS_CLASS(class);
> > pbc->bus_num = pxb_bus_num;
> > - pbc->is_root = pxb_is_root;
> > pbc->numa_node = pxb_bus_numa_node;
> > }
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 6e11dc2fec..5fab7f23b3 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -126,14 +126,9 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
> > vmstate_unregister(NULL, &vmstate_pcibus, bus);
> > }
> > -static bool pcibus_is_root(PCIBus *bus)
> > -{
> > - return !bus->parent_dev;
> > -}
> > -
> > static int pcibus_num(PCIBus *bus)
> > {
> > - if (pcibus_is_root(bus)) {
> > + if (pci_bus_is_root(bus)) {
> > return 0; /* pci host bridge */
> > }
> > return bus->parent_dev->config[PCI_SECONDARY_BUS];
> > @@ -156,7 +151,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
> > k->unrealize = pci_bus_unrealize;
> > k->reset = pcibus_reset;
> > - pbc->is_root = pcibus_is_root;
> > pbc->bus_num = pcibus_num;
> > pbc->numa_node = pcibus_numa_node;
> > }
> > @@ -385,6 +379,7 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
> > bus->slot_reserved_mask = 0x0;
> > bus->address_space_mem = address_space_mem;
> > bus->address_space_io = address_space_io;
> > + bus->flags |= PCI_BUS_IS_ROOT;
> > /* host bridge */
> > QLIST_INIT(&bus->child);
> > @@ -397,11 +392,6 @@ bool pci_bus_is_express(PCIBus *bus)
> > return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
> > }
> > -bool pci_bus_is_root(PCIBus *bus)
> > -{
> > - return PCI_BUS_GET_CLASS(bus)->is_root(bus);
> > -}
> > -
> > void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > const char *name,
> > MemoryRegion *address_space_mem,
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 77d92a3dc4..cbb3386207 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> > #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
> > #define TYPE_PCIE_BUS "PCIE"
> > +enum PCIBusFlags {
> > + /* This bus is the root of a PCI domain */
> > + PCI_BUS_IS_ROOT = 0x0001,
> > +};
> > +
> > typedef struct PCIBusClass {
> > /*< private >*/
> > BusClass parent_class;
> > @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
> > struct PCIBus {
> > BusState qbus;
> > + enum PCIBusFlags flags;
> > PCIIOMMUFunc iommu_fn;
> > void *iommu_opaque;
> > uint8_t devfn_min;
> > @@ -440,8 +446,12 @@ struct PCIBus {
> > Notifier machine_done;
> > };
> > +static inline bool pci_bus_is_root(PCIBus *bus)
> > +{
> > + return !!(bus->flags & PCI_BUS_IS_ROOT);
> > +}
> > +
> > bool pci_bus_is_express(PCIBus *bus);
> > -bool pci_bus_is_root(PCIBus *bus);
> > void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
> > const char *name,
> > MemoryRegion *address_space_mem,
> >
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
On 29/11/2017 15:12, David Gibson wrote:
> On Wed, Nov 29, 2017 at 12:45:28PM +0200, Marcel Apfelbaum wrote:
>> On 29/11/2017 10:46, David Gibson wrote:
>>> pci_bus_is_root() currently relies on a method in the PCIBusClass.
>>> But it's always known if a PCI bus is a root bus when we create it, so
>>> using a dynamic method is overkill.
>>>
>>> This replaces it with an IS_ROOT bit in a new flags field, which is set on
>>> root buses and otherwise clear. As a bonus this removes the special
>>> is_root logic from pci_expander_bridge, since it already creates its bus
>>> as a root bus.
>>>
>>
>> I agree it makes the code simpler, I am not sure about
>> the enum. Why not make it a boolean field and when will need
>> more flags just convert it to an enum (or not)?
>
> Well, I have some patches I'm working on (rather slowly) which add a
> second flag.
>
OK
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
>>
>> Thanks,
>> Marcel
>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>> ---
>>> hw/pci-bridge/pci_expander_bridge.c | 6 ------
>>> hw/pci/pci.c | 14 ++------------
>>> include/hw/pci/pci.h | 12 +++++++++++-
>>> 3 files changed, 13 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/hw/pci-bridge/pci_expander_bridge.c b/hw/pci-bridge/pci_expander_bridge.c
>>> index 5652cf06e9..11dfa9258e 100644
>>> --- a/hw/pci-bridge/pci_expander_bridge.c
>>> +++ b/hw/pci-bridge/pci_expander_bridge.c
>>> @@ -65,11 +65,6 @@ static int pxb_bus_num(PCIBus *bus)
>>> return pxb->bus_nr;
>>> }
>>> -static bool pxb_is_root(PCIBus *bus)
>>> -{
>>> - return true; /* by definition */
>>> -}
>>> -
>>> static uint16_t pxb_bus_numa_node(PCIBus *bus)
>>> {
>>> PXBDev *pxb = convert_to_pxb(bus->parent_dev);
>>> @@ -82,7 +77,6 @@ static void pxb_bus_class_init(ObjectClass *class, void *data)
>>> PCIBusClass *pbc = PCI_BUS_CLASS(class);
>>> pbc->bus_num = pxb_bus_num;
>>> - pbc->is_root = pxb_is_root;
>>> pbc->numa_node = pxb_bus_numa_node;
>>> }
>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>> index 6e11dc2fec..5fab7f23b3 100644
>>> --- a/hw/pci/pci.c
>>> +++ b/hw/pci/pci.c
>>> @@ -126,14 +126,9 @@ static void pci_bus_unrealize(BusState *qbus, Error **errp)
>>> vmstate_unregister(NULL, &vmstate_pcibus, bus);
>>> }
>>> -static bool pcibus_is_root(PCIBus *bus)
>>> -{
>>> - return !bus->parent_dev;
>>> -}
>>> -
>>> static int pcibus_num(PCIBus *bus)
>>> {
>>> - if (pcibus_is_root(bus)) {
>>> + if (pci_bus_is_root(bus)) {
>>> return 0; /* pci host bridge */
>>> }
>>> return bus->parent_dev->config[PCI_SECONDARY_BUS];
>>> @@ -156,7 +151,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data)
>>> k->unrealize = pci_bus_unrealize;
>>> k->reset = pcibus_reset;
>>> - pbc->is_root = pcibus_is_root;
>>> pbc->bus_num = pcibus_num;
>>> pbc->numa_node = pcibus_numa_node;
>>> }
>>> @@ -385,6 +379,7 @@ static void pci_root_bus_init(PCIBus *bus, DeviceState *parent,
>>> bus->slot_reserved_mask = 0x0;
>>> bus->address_space_mem = address_space_mem;
>>> bus->address_space_io = address_space_io;
>>> + bus->flags |= PCI_BUS_IS_ROOT;
>>> /* host bridge */
>>> QLIST_INIT(&bus->child);
>>> @@ -397,11 +392,6 @@ bool pci_bus_is_express(PCIBus *bus)
>>> return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS);
>>> }
>>> -bool pci_bus_is_root(PCIBus *bus)
>>> -{
>>> - return PCI_BUS_GET_CLASS(bus)->is_root(bus);
>>> -}
>>> -
>>> void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>>> const char *name,
>>> MemoryRegion *address_space_mem,
>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>> index 77d92a3dc4..cbb3386207 100644
>>> --- a/include/hw/pci/pci.h
>>> +++ b/include/hw/pci/pci.h
>>> @@ -404,6 +404,11 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
>>> #define PCI_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(PCIBusClass, (obj), TYPE_PCI_BUS)
>>> #define TYPE_PCIE_BUS "PCIE"
>>> +enum PCIBusFlags {
>>> + /* This bus is the root of a PCI domain */
>>> + PCI_BUS_IS_ROOT = 0x0001,
>>> +};
>>> +
>>> typedef struct PCIBusClass {
>>> /*< private >*/
>>> BusClass parent_class;
>>> @@ -416,6 +421,7 @@ typedef struct PCIBusClass {
>>> struct PCIBus {
>>> BusState qbus;
>>> + enum PCIBusFlags flags;
>>> PCIIOMMUFunc iommu_fn;
>>> void *iommu_opaque;
>>> uint8_t devfn_min;
>>> @@ -440,8 +446,12 @@ struct PCIBus {
>>> Notifier machine_done;
>>> };
>>> +static inline bool pci_bus_is_root(PCIBus *bus)
>>> +{
>>> + return !!(bus->flags & PCI_BUS_IS_ROOT);
>>> +}
>>> +
>>> bool pci_bus_is_express(PCIBus *bus);
>>> -bool pci_bus_is_root(PCIBus *bus);
>>> void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
>>> const char *name,
>>> MemoryRegion *address_space_mem,
>>>
>>
>
On Wed, Nov 29, 2017 at 07:46:25PM +1100, David Gibson wrote:
[...]
> struct PCIBus {
> BusState qbus;
> + enum PCIBusFlags flags;
> PCIIOMMUFunc iommu_fn;
> void *iommu_opaque;
> uint8_t devfn_min;
> @@ -440,8 +446,12 @@ struct PCIBus {
> Notifier machine_done;
> };
I would prefer directly defining flags as uint64_t then it's more
clear that it's a bitmask (enum let me think of only one flag will be
set, but I may be wrong), but it's fine too to me.
Reviewed-by: Peter Xu <peterx@redhat.com>
--
Peter Xu
On Thu, Nov 30, 2017 at 02:23:45PM +0800, Peter Xu wrote:
> On Wed, Nov 29, 2017 at 07:46:25PM +1100, David Gibson wrote:
>
> [...]
>
> > struct PCIBus {
> > BusState qbus;
> > + enum PCIBusFlags flags;
> > PCIIOMMUFunc iommu_fn;
> > void *iommu_opaque;
> > uint8_t devfn_min;
> > @@ -440,8 +446,12 @@ struct PCIBus {
> > Notifier machine_done;
> > };
>
> I would prefer directly defining flags as uint64_t then it's more
> clear that it's a bitmask (enum let me think of only one flag will be
> set, but I may be wrong), but it's fine too to me.
In most languages an enum would only allow one value to be set, but C
enums are weird - they're basically just a funny way of defining
integer constants.
I'm not particularly bothered either way: using the enum type makes it
clear the right set of constants to use with this field, using an int
makes it clearer that bit flags can be set. So both options are
slightly misleading in different ways.
If there's another vote for uint64_t over using the enum, I'll change
it.
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
© 2016 - 2026 Red Hat, Inc.