Philippe Mathieu-Daudé <[email protected]> writes: > On 2/2/26 12:50, Alex Bennée wrote: >> Philippe Mathieu-Daudé <[email protected]> writes: >> >>> The C99 standard chapter §6.7.2.2 point 4 mentions: >>> >>> Each enumerated type shall be compatible with char, a signed >>> integer type, or an unsigned integer type. The choice of type >>> is implementation-defined, but shall be capable of representing >>> the values of all the members of the enumeration. >>> >>> Align with that recommendation by defining the typedef >>> along with the enum. >>> >>> For information, building with -Werror=pedantic was reporting: >>> >>> In file included from ../../ui/kbd-state.c:10: >>> include/ui/kbd-state.h:12:14: error: ISO C forbids forward references to >>> 'enum' types [-Werror,-Wpedantic] >>> 12 | typedef enum QKbdModifier QKbdModifier; >>> | ^ >>> ... >>> >>> Reported-by: Roman Kiryanov <[email protected]> >>> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> >>> Reviewed-by: Pierrick Bouvier <[email protected]> >>> --- >>> hw/riscv/riscv-iommu-bits.h | 4 ++-- >>> hw/riscv/riscv-iommu.h | 2 -- >>> include/hw/misc/auxbus.h | 11 ++++------- >>> include/hw/pci/pci_device.h | 5 ++--- >>> include/hw/ssi/ssi.h | 6 ++---- >>> include/hw/xen/interface/io/xenbus.h | 5 ++--- >>> include/io/channel.h | 11 ++++------- >>> include/system/replay.h | 10 ++++------ >>> include/ui/clipboard.h | 15 ++++++--------- >>> include/ui/kbd-state.h | 6 ++---- >>> pc-bios/s390-ccw/virtio.h | 5 ++--- >>> tests/qtest/libqos/qgraph_internal.h | 10 ++++------ >>> hw/core/loader.c | 5 ++--- >>> hw/display/xlnx_dp.c | 11 ++++------- >>> hw/dma/xlnx_dpdma.c | 10 ++++------ >>> qapi/opts-visitor.c | 7 ++----- >>> qapi/string-output-visitor.c | 6 ++---- >>> tests/unit/check-qom-proplist.c | 6 ++---- >>> 18 files changed, 50 insertions(+), 85 deletions(-) >>> >>> diff --git a/hw/riscv/riscv-iommu-bits.h b/hw/riscv/riscv-iommu-bits.h >>> index 47fe01bee58..281afa7bc3f 100644 >>> --- a/hw/riscv/riscv-iommu-bits.h >>> +++ b/hw/riscv/riscv-iommu-bits.h >>> @@ -96,11 +96,11 @@ struct riscv_iommu_pq_record { >>> #define RISCV_IOMMU_CAP_PD17 BIT_ULL(39) >>> #define RISCV_IOMMU_CAP_PD20 BIT_ULL(40) >>> -enum riscv_iommu_igs_modes { >>> +typedef enum riscv_iommu_igs_modes { >>> RISCV_IOMMU_CAP_IGS_MSI = 0, >>> RISCV_IOMMU_CAP_IGS_WSI, >>> RISCV_IOMMU_CAP_IGS_BOTH >>> -}; >>> +} riscv_iommu_igs_mode; >> mode vs modes. > > Pre-existing... > >> It seems very clumsy to have the type at the front and >> back of the declaration. The only real reason to have the tag ahead of >> the struct/enum was if there are references in the structure. >> IOW this does feel like a lot of churn. > > Well I got it clear old-school maintainers are very reluctant > to open the project to more contributors, even if changes > required are only cosmetics, and less invasive than Rust ones.
Is this opening up to more contributors? As far as I can tell this seems to be trying to make life easier for downstream for something we don't want contributed to the upstream (C++). > I won't put more energy to this now. Maybe in 10 years from > now if the project still exists, and we get more inclusive. I'm all for improving inclusively and lowering barriers to entry for contributors upstream but I'm not sure how this does that. > > Regards, > > Phil. -- Alex Bennée Virtualisation Tech Lead @ Linaro
