* [PATCH v2 0/2] bulk: Stop forward-declaring enum typedefs @ 2026-01-30 21:17 Philippe Mathieu-Daudé 2026-01-30 21:17 ` [PATCH v2 1/2] " Philippe Mathieu-Daudé 2026-01-30 21:17 ` [PATCH v2 2/2] docs/devel: Mention enum typedefs forward-declaration is not allowed Philippe Mathieu-Daudé 0 siblings, 2 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2026-01-30 21:17 UTC (permalink / raw) To: qemu-devel Cc: jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake v2: - Fixed QemuClipboardNotifyType typo - checkpatch regexp - Pierrick's R-b 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. Philippe Mathieu-Daudé (2): bulk: Stop forward-declaring enum typedefs docs/devel: Mention enum typedefs forward-declaration is not allowed docs/devel/style.rst | 11 +++++++++++ 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 ++---- scripts/checkpatch.pl | 5 +++++ 20 files changed, 66 insertions(+), 85 deletions(-) -- 2.52.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-01-30 21:17 [PATCH v2 0/2] bulk: Stop forward-declaring enum typedefs Philippe Mathieu-Daudé @ 2026-01-30 21:17 ` Philippe Mathieu-Daudé 2026-02-02 11:19 ` Daniel P. Berrangé 2026-02-02 11:50 ` Alex Bennée 2026-01-30 21:17 ` [PATCH v2 2/2] docs/devel: Mention enum typedefs forward-declaration is not allowed Philippe Mathieu-Daudé 1 sibling, 2 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2026-01-30 21:17 UTC (permalink / raw) To: qemu-devel Cc: jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake 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 <rkir@google.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- 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; /* 5.4 Features control register (32bits) */ #define RISCV_IOMMU_REG_FCTL 0x0008 diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h index 2dabd86941b..fbf70fb81e1 100644 --- a/hw/riscv/riscv-iommu.h +++ b/hw/riscv/riscv-iommu.h @@ -25,8 +25,6 @@ #include "hw/riscv/iommu.h" #include "hw/riscv/riscv-iommu-bits.h" -typedef enum riscv_iommu_igs_modes riscv_iommu_igs_mode; - struct RISCVIOMMUState { /*< private >*/ DeviceState parent_obj; diff --git a/include/hw/misc/auxbus.h b/include/hw/misc/auxbus.h index 5bfdd05e152..11f1a87d44f 100644 --- a/include/hw/misc/auxbus.h +++ b/include/hw/misc/auxbus.h @@ -29,16 +29,13 @@ #include "hw/core/qdev.h" #include "qom/object.h" -typedef enum AUXCommand AUXCommand; -typedef enum AUXReply AUXReply; - #define TYPE_AUXTOI2C "aux-to-i2c-bridge" OBJECT_DECLARE_SIMPLE_TYPE(AUXTOI2CState, AUXTOI2C) #define TYPE_AUX_SLAVE "aux-slave" OBJECT_DECLARE_SIMPLE_TYPE(AUXSlave, AUX_SLAVE) -enum AUXCommand { +typedef enum AUXCommand { WRITE_I2C = 0, READ_I2C = 1, WRITE_I2C_STATUS = 2, @@ -46,15 +43,15 @@ enum AUXCommand { READ_I2C_MOT = 5, WRITE_AUX = 8, READ_AUX = 9 -}; +} AUXCommand; -enum AUXReply { +typedef enum AUXReply { AUX_I2C_ACK = 0, AUX_NACK = 1, AUX_DEFER = 2, AUX_I2C_NACK = 4, AUX_I2C_DEFER = 8 -}; +} AUXReply; #define TYPE_AUX_BUS "aux-bus" OBJECT_DECLARE_SIMPLE_TYPE(AUXBus, AUX_BUS) diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h index 88ccea50113..85e57fa5ca1 100644 --- a/include/hw/pci/pci_device.h +++ b/include/hw/pci/pci_device.h @@ -43,13 +43,12 @@ struct PCIDeviceClass { bool sriov_vf_user_creatable; }; -enum PCIReqIDType { +typedef enum PCIReqIDType { PCI_REQ_ID_INVALID = 0, PCI_REQ_ID_BDF, PCI_REQ_ID_SECONDARY_BUS, PCI_REQ_ID_MAX, -}; -typedef enum PCIReqIDType PCIReqIDType; +} PCIReqIDType; struct PCIReqIDCache { PCIDevice *dev; diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h index 6d6d8ccb3d4..d576e8cb061 100644 --- a/include/hw/ssi/ssi.h +++ b/include/hw/ssi/ssi.h @@ -16,19 +16,17 @@ #include "hw/core/qdev.h" #include "qom/object.h" -typedef enum SSICSMode SSICSMode; - #define TYPE_SSI_PERIPHERAL "ssi-peripheral" OBJECT_DECLARE_TYPE(SSIPeripheral, SSIPeripheralClass, SSI_PERIPHERAL) #define SSI_GPIO_CS "ssi-gpio-cs" -enum SSICSMode { +typedef enum SSICSMode { SSI_CS_NONE = 0, SSI_CS_LOW, SSI_CS_HIGH, -}; +} SSICSMode; /* Peripherals. */ struct SSIPeripheralClass { diff --git a/include/hw/xen/interface/io/xenbus.h b/include/hw/xen/interface/io/xenbus.h index 9cd0cd7c670..3fe2f53c22c 100644 --- a/include/hw/xen/interface/io/xenbus.h +++ b/include/hw/xen/interface/io/xenbus.h @@ -16,7 +16,7 @@ * the state of the connection between the driver and the kernel's device * layers. */ -enum xenbus_state { +typedef enum xenbus_state { XenbusStateUnknown = 0, XenbusStateInitialising = 1, @@ -47,8 +47,7 @@ enum xenbus_state { XenbusStateReconfiguring = 7, XenbusStateReconfigured = 8 -}; -typedef enum xenbus_state XenbusState; +} XenbusState; #endif /* _XEN_PUBLIC_IO_XENBUS_H */ diff --git a/include/io/channel.h b/include/io/channel.h index 1b02350437d..fb6ec407a91 100644 --- a/include/io/channel.h +++ b/include/io/channel.h @@ -38,9 +38,8 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass, #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2 #define QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING 0x4 -typedef enum QIOChannelFeature QIOChannelFeature; -enum QIOChannelFeature { +typedef enum QIOChannelFeature { QIO_CHANNEL_FEATURE_FD_PASS, QIO_CHANNEL_FEATURE_SHUTDOWN, QIO_CHANNEL_FEATURE_LISTEN, @@ -48,16 +47,14 @@ enum QIOChannelFeature { QIO_CHANNEL_FEATURE_READ_MSG_PEEK, QIO_CHANNEL_FEATURE_SEEKABLE, QIO_CHANNEL_FEATURE_CONCURRENT_IO, -}; +} QIOChannelFeature; -typedef enum QIOChannelShutdown QIOChannelShutdown; - -enum QIOChannelShutdown { +typedef enum QIOChannelShutdown { QIO_CHANNEL_SHUTDOWN_READ = 1, QIO_CHANNEL_SHUTDOWN_WRITE = 2, QIO_CHANNEL_SHUTDOWN_BOTH = 3, -}; +} QIOChannelShutdown; typedef gboolean (*QIOChannelFunc)(QIOChannel *ioc, GIOCondition condition, diff --git a/include/system/replay.h b/include/system/replay.h index f8715ca9feb..ed34256d8bb 100644 --- a/include/system/replay.h +++ b/include/system/replay.h @@ -19,17 +19,16 @@ #include "qemu/audio.h" /* replay clock kinds */ -enum ReplayClockKind { +typedef enum ReplayClockKind { /* host_clock */ REPLAY_CLOCK_HOST, /* virtual_rt_clock */ REPLAY_CLOCK_VIRTUAL_RT, REPLAY_CLOCK_COUNT -}; -typedef enum ReplayClockKind ReplayClockKind; +} ReplayClockKind; /* IDs of the checkpoints */ -enum ReplayCheckpoint { +typedef enum ReplayCheckpoint { CHECKPOINT_CLOCK_WARP_START, CHECKPOINT_CLOCK_WARP_ACCOUNT, CHECKPOINT_RESET_REQUESTED, @@ -40,8 +39,7 @@ enum ReplayCheckpoint { CHECKPOINT_INIT, CHECKPOINT_RESET, CHECKPOINT_COUNT -}; -typedef enum ReplayCheckpoint ReplayCheckpoint; +} ReplayCheckpoint; typedef struct ReplayNetState ReplayNetState; diff --git a/include/ui/clipboard.h b/include/ui/clipboard.h index 62a96ce9ff5..2704434ded9 100644 --- a/include/ui/clipboard.h +++ b/include/ui/clipboard.h @@ -20,9 +20,6 @@ * is supported for now. */ -typedef enum QemuClipboardType QemuClipboardType; -typedef enum QemuClipboardNotifyType QemuClipboardNotifyType; -typedef enum QemuClipboardSelection QemuClipboardSelection; typedef struct QemuClipboardPeer QemuClipboardPeer; typedef struct QemuClipboardNotify QemuClipboardNotify; typedef struct QemuClipboardInfo QemuClipboardInfo; @@ -36,10 +33,10 @@ extern const VMStateDescription vmstate_cbinfo; * @QEMU_CLIPBOARD_TYPE_TEXT: text/plain; charset=utf-8 * @QEMU_CLIPBOARD_TYPE__COUNT: type count. */ -enum QemuClipboardType { +typedef enum QemuClipboardType { QEMU_CLIPBOARD_TYPE_TEXT, QEMU_CLIPBOARD_TYPE__COUNT, -}; +} QemuClipboardType; /* same as VD_AGENT_CLIPBOARD_SELECTION_* */ /** @@ -50,12 +47,12 @@ enum QemuClipboardType { * @QEMU_CLIPBOARD_SELECTION_SECONDARY: secondary selection (dunno). * @QEMU_CLIPBOARD_SELECTION__COUNT: selection count. */ -enum QemuClipboardSelection { +typedef enum QemuClipboardSelection { QEMU_CLIPBOARD_SELECTION_CLIPBOARD, QEMU_CLIPBOARD_SELECTION_PRIMARY, QEMU_CLIPBOARD_SELECTION_SECONDARY, QEMU_CLIPBOARD_SELECTION__COUNT, -}; +} QemuClipboardSelection; /** * struct QemuClipboardPeer @@ -81,10 +78,10 @@ struct QemuClipboardPeer { * * Clipboard notify type. */ -enum QemuClipboardNotifyType { +typedef enum QemuClipboardNotifyType { QEMU_CLIPBOARD_UPDATE_INFO, QEMU_CLIPBOARD_RESET_SERIAL, -}; +} QemuClipboardNotifyType; /** * struct QemuClipboardNotify diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h index 1f37b932eb6..3522ebe51b9 100644 --- a/include/ui/kbd-state.h +++ b/include/ui/kbd-state.h @@ -9,9 +9,7 @@ #include "qapi/qapi-types-ui.h" -typedef enum QKbdModifier QKbdModifier; - -enum QKbdModifier { +typedef enum QKbdModifier { QKBD_MOD_NONE = 0, QKBD_MOD_SHIFT, @@ -23,7 +21,7 @@ enum QKbdModifier { QKBD_MOD_CAPSLOCK, QKBD_MOD__MAX -}; +} QKbdModifier; typedef struct QKbdState QKbdState; diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h index 5c5e808a500..67fd0da9fd2 100644 --- a/pc-bios/s390-ccw/virtio.h +++ b/pc-bios/s390-ccw/virtio.h @@ -173,13 +173,12 @@ struct VirtioBlkConfig { } __attribute__((packed)); typedef struct VirtioBlkConfig VirtioBlkConfig; -enum guessed_disk_nature_type { +typedef enum guessed_disk_nature_type { VIRTIO_GDN_NONE = 0, VIRTIO_GDN_DASD = 1, VIRTIO_GDN_CDROM = 2, VIRTIO_GDN_SCSI = 3, -}; -typedef enum guessed_disk_nature_type VirtioGDN; +} VirtioGDN; VirtioGDN virtio_guessed_disk_nature(void); void virtio_assume_eckd(void); diff --git a/tests/qtest/libqos/qgraph_internal.h b/tests/qtest/libqos/qgraph_internal.h index 87fab1f9f0d..e2e740a59db 100644 --- a/tests/qtest/libqos/qgraph_internal.h +++ b/tests/qtest/libqos/qgraph_internal.h @@ -27,8 +27,6 @@ #include "qgraph.h" typedef struct QOSGraphMachine QOSGraphMachine; -typedef enum QOSEdgeType QOSEdgeType; -typedef enum QOSNodeType QOSNodeType; /* callback called when the walk path algorithm found a * valid path @@ -36,19 +34,19 @@ typedef enum QOSNodeType QOSNodeType; typedef void (*QOSTestCallback) (QOSGraphNode *path, int len); /* edge types*/ -enum QOSEdgeType { +typedef enum QOSEdgeType { QEDGE_CONTAINS, QEDGE_PRODUCES, QEDGE_CONSUMED_BY -}; +} QOSEdgeType; /* node types*/ -enum QOSNodeType { +typedef enum QOSNodeType { QNODE_MACHINE, QNODE_DRIVER, QNODE_INTERFACE, QNODE_TEST -}; +} QOSNodeType; /* Graph Node */ struct QOSGraphNode { diff --git a/hw/core/loader.c b/hw/core/loader.c index 5cbfba0a86d..5b6f022428c 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -1688,15 +1688,14 @@ HumanReadableText *qmp_x_query_roms(Error **errp) return human_readable_text_from_str(buf); } -typedef enum HexRecord HexRecord; -enum HexRecord { +typedef enum HexRecord { DATA_RECORD = 0, EOF_RECORD, EXT_SEG_ADDR_RECORD, START_SEG_ADDR_RECORD, EXT_LINEAR_ADDR_RECORD, START_LINEAR_ADDR_RECORD, -}; +} HexRecord; /* Each record contains a 16-bit address which is combined with the upper 16 * bits of the implicit "next address" to form a 32-bit address. diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c index a24e95f0a73..1ab2375625c 100644 --- a/hw/display/xlnx_dp.c +++ b/hw/display/xlnx_dp.c @@ -212,7 +212,7 @@ #define DP_GRAPHIC_DMA_CHANNEL (3) #define DP_VIDEO_DMA_CHANNEL (0) -enum DPGraphicFmt { +typedef enum DPGraphicFmt { DP_GRAPHIC_RGBA8888 = 0 << 8, DP_GRAPHIC_ABGR8888 = 1 << 8, DP_GRAPHIC_RGB888 = 2 << 8, @@ -225,9 +225,9 @@ enum DPGraphicFmt { DP_GRAPHIC_2BPP = 9 << 8, DP_GRAPHIC_1BPP = 10 << 8, DP_GRAPHIC_MASK = 0xF << 8 -}; +} DPGraphicFmt; -enum DPVideoFmt { +typedef enum DPVideoFmt { DP_NL_VID_CB_Y0_CR_Y1 = 0, DP_NL_VID_CR_Y0_CB_Y1 = 1, DP_NL_VID_Y0_CR_Y1_CB = 2, @@ -254,10 +254,7 @@ enum DPVideoFmt { DP_NL_VID_YV16CL_420_10BPC = 23, DP_NL_VID_YV16CL2_420_10BPC = 24, DP_NL_VID_FMT_MASK = 0x1F -}; - -typedef enum DPGraphicFmt DPGraphicFmt; -typedef enum DPVideoFmt DPVideoFmt; +} DPVideoFmt; static const VMStateDescription vmstate_dp = { .name = TYPE_XLNX_DP, diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c index 32f56036a4c..ef5488b3eca 100644 --- a/hw/dma/xlnx_dpdma.c +++ b/hw/dma/xlnx_dpdma.c @@ -122,15 +122,15 @@ #define DPDMA_FRAG_MAX_SZ (4096) -enum DPDMABurstType { +typedef enum DPDMABurstType { DPDMA_INCR = 0, DPDMA_FIXED = 1 -}; +} DPDMABurstType; -enum DPDMAMode { +typedef enum DPDMAMode { DPDMA_CONTIGOUS = 0, DPDMA_FRAGMENTED = 1 -}; +} DPDMAMode; struct DPDMADescriptor { uint32_t control; @@ -153,8 +153,6 @@ struct DPDMADescriptor { uint32_t crc; }; -typedef enum DPDMABurstType DPDMABurstType; -typedef enum DPDMAMode DPDMAMode; typedef struct DPDMADescriptor DPDMADescriptor; static bool xlnx_dpdma_desc_is_last(DPDMADescriptor *desc) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 3d1a28b4191..66b1b931a31 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -20,8 +20,7 @@ #include "qapi/visitor-impl.h" -enum ListMode -{ +typedef enum ListMode { LM_NONE, /* not traversing a list of repeated options */ LM_IN_PROGRESS, /* @@ -58,9 +57,7 @@ enum ListMode * No more QemuOpt instance in the list. * The traversal has been completed. */ -}; - -typedef enum ListMode ListMode; +} ListMode; struct OptsVisitor { diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c index 5115536b153..5d175773356 100644 --- a/qapi/string-output-visitor.c +++ b/qapi/string-output-visitor.c @@ -17,7 +17,7 @@ #include <math.h> #include "qemu/range.h" -enum ListMode { +typedef enum ListMode { LM_NONE, /* not traversing a list of repeated options */ LM_STARTED, /* next_list() ready to be called */ @@ -48,9 +48,7 @@ enum ListMode { LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */ LM_END, /* next_list() called, about to see last element. */ -}; - -typedef enum ListMode ListMode; +} ListMode; struct StringOutputVisitor { diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c index ee3c6fb32b1..6f3dbc4407a 100644 --- a/tests/unit/check-qom-proplist.c +++ b/tests/unit/check-qom-proplist.c @@ -40,15 +40,13 @@ typedef struct DummyObjectClass DummyObjectClass; DECLARE_INSTANCE_CHECKER(DummyObject, DUMMY_OBJECT, TYPE_DUMMY) -typedef enum DummyAnimal DummyAnimal; - -enum DummyAnimal { +typedef enum DummyAnimal { DUMMY_FROG, DUMMY_ALLIGATOR, DUMMY_PLATYPUS, DUMMY_LAST, -}; +} DummyAnimal; const QEnumLookup dummy_animal_map = { .array = (const char *const[]) { -- 2.52.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-01-30 21:17 ` [PATCH v2 1/2] " Philippe Mathieu-Daudé @ 2026-02-02 11:19 ` Daniel P. Berrangé 2026-02-02 11:50 ` Alex Bennée 1 sibling, 0 replies; 8+ messages in thread From: Daniel P. Berrangé @ 2026-02-02 11:19 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake On Fri, Jan 30, 2026 at 10:17:45PM +0100, Philippe Mathieu-Daudé wrote: > 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. What am I missing here - that text isn't talking about forward declarations / typedefs ? It is just saying that the size of an enum can vary betweeen compilers. > 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; > | ^ > ... Compliance with -Wpedantic is a non-goal for QEMU. We long ago declared that our goal was only to support GCC/CLang, with the GNU dialect of C, explicitly *NOT* strict compliance with ISO C. > diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h > index 1f37b932eb6..3522ebe51b9 100644 > --- a/include/ui/kbd-state.h > +++ b/include/ui/kbd-state.h > @@ -9,9 +9,7 @@ > > #include "qapi/qapi-types-ui.h" > > -typedef enum QKbdModifier QKbdModifier; Note this style... > - > -enum QKbdModifier { > +typedef enum QKbdModifier { > QKBD_MOD_NONE = 0, > > QKBD_MOD_SHIFT, > @@ -23,7 +21,7 @@ enum QKbdModifier { > QKBD_MOD_CAPSLOCK, > > QKBD_MOD__MAX > -}; > +} QKbdModifier; > > typedef struct QKbdState QKbdState; ...is a match for this style we use with structs, so this patch is forcing divergence in styles. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-01-30 21:17 ` [PATCH v2 1/2] " Philippe Mathieu-Daudé 2026-02-02 11:19 ` Daniel P. Berrangé @ 2026-02-02 11:50 ` Alex Bennée 2026-02-02 12:05 ` Philippe Mathieu-Daudé 1 sibling, 1 reply; 8+ messages in thread From: Alex Bennée @ 2026-02-02 11:50 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake Philippe Mathieu-Daudé <philmd@linaro.org> 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 <rkir@google.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > --- > 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. 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. > > /* 5.4 Features control register (32bits) */ > #define RISCV_IOMMU_REG_FCTL 0x0008 > diff --git a/hw/riscv/riscv-iommu.h b/hw/riscv/riscv-iommu.h > index 2dabd86941b..fbf70fb81e1 100644 > --- a/hw/riscv/riscv-iommu.h > +++ b/hw/riscv/riscv-iommu.h > @@ -25,8 +25,6 @@ > #include "hw/riscv/iommu.h" > #include "hw/riscv/riscv-iommu-bits.h" > > -typedef enum riscv_iommu_igs_modes riscv_iommu_igs_mode; > - > struct RISCVIOMMUState { > /*< private >*/ > DeviceState parent_obj; > diff --git a/include/hw/misc/auxbus.h b/include/hw/misc/auxbus.h > index 5bfdd05e152..11f1a87d44f 100644 > --- a/include/hw/misc/auxbus.h > +++ b/include/hw/misc/auxbus.h > @@ -29,16 +29,13 @@ > #include "hw/core/qdev.h" > #include "qom/object.h" > > -typedef enum AUXCommand AUXCommand; > -typedef enum AUXReply AUXReply; > - > #define TYPE_AUXTOI2C "aux-to-i2c-bridge" > OBJECT_DECLARE_SIMPLE_TYPE(AUXTOI2CState, AUXTOI2C) > > #define TYPE_AUX_SLAVE "aux-slave" > OBJECT_DECLARE_SIMPLE_TYPE(AUXSlave, AUX_SLAVE) > > -enum AUXCommand { > +typedef enum AUXCommand { > WRITE_I2C = 0, > READ_I2C = 1, > WRITE_I2C_STATUS = 2, > @@ -46,15 +43,15 @@ enum AUXCommand { > READ_I2C_MOT = 5, > WRITE_AUX = 8, > READ_AUX = 9 > -}; > +} AUXCommand; > > -enum AUXReply { > +typedef enum AUXReply { > AUX_I2C_ACK = 0, > AUX_NACK = 1, > AUX_DEFER = 2, > AUX_I2C_NACK = 4, > AUX_I2C_DEFER = 8 > -}; > +} AUXReply; > > #define TYPE_AUX_BUS "aux-bus" > OBJECT_DECLARE_SIMPLE_TYPE(AUXBus, AUX_BUS) > diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h > index 88ccea50113..85e57fa5ca1 100644 > --- a/include/hw/pci/pci_device.h > +++ b/include/hw/pci/pci_device.h > @@ -43,13 +43,12 @@ struct PCIDeviceClass { > bool sriov_vf_user_creatable; > }; > > -enum PCIReqIDType { > +typedef enum PCIReqIDType { > PCI_REQ_ID_INVALID = 0, > PCI_REQ_ID_BDF, > PCI_REQ_ID_SECONDARY_BUS, > PCI_REQ_ID_MAX, > -}; > -typedef enum PCIReqIDType PCIReqIDType; > +} PCIReqIDType; > > struct PCIReqIDCache { > PCIDevice *dev; > diff --git a/include/hw/ssi/ssi.h b/include/hw/ssi/ssi.h > index 6d6d8ccb3d4..d576e8cb061 100644 > --- a/include/hw/ssi/ssi.h > +++ b/include/hw/ssi/ssi.h > @@ -16,19 +16,17 @@ > #include "hw/core/qdev.h" > #include "qom/object.h" > > -typedef enum SSICSMode SSICSMode; > - > #define TYPE_SSI_PERIPHERAL "ssi-peripheral" > OBJECT_DECLARE_TYPE(SSIPeripheral, SSIPeripheralClass, > SSI_PERIPHERAL) > > #define SSI_GPIO_CS "ssi-gpio-cs" > > -enum SSICSMode { > +typedef enum SSICSMode { > SSI_CS_NONE = 0, > SSI_CS_LOW, > SSI_CS_HIGH, > -}; > +} SSICSMode; > > /* Peripherals. */ > struct SSIPeripheralClass { > diff --git a/include/hw/xen/interface/io/xenbus.h b/include/hw/xen/interface/io/xenbus.h > index 9cd0cd7c670..3fe2f53c22c 100644 > --- a/include/hw/xen/interface/io/xenbus.h > +++ b/include/hw/xen/interface/io/xenbus.h > @@ -16,7 +16,7 @@ > * the state of the connection between the driver and the kernel's device > * layers. > */ > -enum xenbus_state { > +typedef enum xenbus_state { > XenbusStateUnknown = 0, > > XenbusStateInitialising = 1, > @@ -47,8 +47,7 @@ enum xenbus_state { > XenbusStateReconfiguring = 7, > > XenbusStateReconfigured = 8 > -}; > -typedef enum xenbus_state XenbusState; > +} XenbusState; > > #endif /* _XEN_PUBLIC_IO_XENBUS_H */ > > diff --git a/include/io/channel.h b/include/io/channel.h > index 1b02350437d..fb6ec407a91 100644 > --- a/include/io/channel.h > +++ b/include/io/channel.h > @@ -38,9 +38,8 @@ OBJECT_DECLARE_TYPE(QIOChannel, QIOChannelClass, > #define QIO_CHANNEL_READ_FLAG_RELAXED_EOF 0x2 > #define QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING 0x4 > > -typedef enum QIOChannelFeature QIOChannelFeature; > > -enum QIOChannelFeature { > +typedef enum QIOChannelFeature { > QIO_CHANNEL_FEATURE_FD_PASS, > QIO_CHANNEL_FEATURE_SHUTDOWN, > QIO_CHANNEL_FEATURE_LISTEN, > @@ -48,16 +47,14 @@ enum QIOChannelFeature { > QIO_CHANNEL_FEATURE_READ_MSG_PEEK, > QIO_CHANNEL_FEATURE_SEEKABLE, > QIO_CHANNEL_FEATURE_CONCURRENT_IO, > -}; > +} QIOChannelFeature; > > > -typedef enum QIOChannelShutdown QIOChannelShutdown; > - > -enum QIOChannelShutdown { > +typedef enum QIOChannelShutdown { > QIO_CHANNEL_SHUTDOWN_READ = 1, > QIO_CHANNEL_SHUTDOWN_WRITE = 2, > QIO_CHANNEL_SHUTDOWN_BOTH = 3, > -}; > +} QIOChannelShutdown; > > typedef gboolean (*QIOChannelFunc)(QIOChannel *ioc, > GIOCondition condition, > diff --git a/include/system/replay.h b/include/system/replay.h > index f8715ca9feb..ed34256d8bb 100644 > --- a/include/system/replay.h > +++ b/include/system/replay.h > @@ -19,17 +19,16 @@ > #include "qemu/audio.h" > > /* replay clock kinds */ > -enum ReplayClockKind { > +typedef enum ReplayClockKind { > /* host_clock */ > REPLAY_CLOCK_HOST, > /* virtual_rt_clock */ > REPLAY_CLOCK_VIRTUAL_RT, > REPLAY_CLOCK_COUNT > -}; > -typedef enum ReplayClockKind ReplayClockKind; > +} ReplayClockKind; > > /* IDs of the checkpoints */ > -enum ReplayCheckpoint { > +typedef enum ReplayCheckpoint { > CHECKPOINT_CLOCK_WARP_START, > CHECKPOINT_CLOCK_WARP_ACCOUNT, > CHECKPOINT_RESET_REQUESTED, > @@ -40,8 +39,7 @@ enum ReplayCheckpoint { > CHECKPOINT_INIT, > CHECKPOINT_RESET, > CHECKPOINT_COUNT > -}; > -typedef enum ReplayCheckpoint ReplayCheckpoint; > +} ReplayCheckpoint; > > typedef struct ReplayNetState ReplayNetState; > > diff --git a/include/ui/clipboard.h b/include/ui/clipboard.h > index 62a96ce9ff5..2704434ded9 100644 > --- a/include/ui/clipboard.h > +++ b/include/ui/clipboard.h > @@ -20,9 +20,6 @@ > * is supported for now. > */ > > -typedef enum QemuClipboardType QemuClipboardType; > -typedef enum QemuClipboardNotifyType QemuClipboardNotifyType; > -typedef enum QemuClipboardSelection QemuClipboardSelection; > typedef struct QemuClipboardPeer QemuClipboardPeer; > typedef struct QemuClipboardNotify QemuClipboardNotify; > typedef struct QemuClipboardInfo QemuClipboardInfo; > @@ -36,10 +33,10 @@ extern const VMStateDescription vmstate_cbinfo; > * @QEMU_CLIPBOARD_TYPE_TEXT: text/plain; charset=utf-8 > * @QEMU_CLIPBOARD_TYPE__COUNT: type count. > */ > -enum QemuClipboardType { > +typedef enum QemuClipboardType { > QEMU_CLIPBOARD_TYPE_TEXT, > QEMU_CLIPBOARD_TYPE__COUNT, > -}; > +} QemuClipboardType; > > /* same as VD_AGENT_CLIPBOARD_SELECTION_* */ > /** > @@ -50,12 +47,12 @@ enum QemuClipboardType { > * @QEMU_CLIPBOARD_SELECTION_SECONDARY: secondary selection (dunno). > * @QEMU_CLIPBOARD_SELECTION__COUNT: selection count. > */ > -enum QemuClipboardSelection { > +typedef enum QemuClipboardSelection { > QEMU_CLIPBOARD_SELECTION_CLIPBOARD, > QEMU_CLIPBOARD_SELECTION_PRIMARY, > QEMU_CLIPBOARD_SELECTION_SECONDARY, > QEMU_CLIPBOARD_SELECTION__COUNT, > -}; > +} QemuClipboardSelection; > > /** > * struct QemuClipboardPeer > @@ -81,10 +78,10 @@ struct QemuClipboardPeer { > * > * Clipboard notify type. > */ > -enum QemuClipboardNotifyType { > +typedef enum QemuClipboardNotifyType { > QEMU_CLIPBOARD_UPDATE_INFO, > QEMU_CLIPBOARD_RESET_SERIAL, > -}; > +} QemuClipboardNotifyType; > > /** > * struct QemuClipboardNotify > diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h > index 1f37b932eb6..3522ebe51b9 100644 > --- a/include/ui/kbd-state.h > +++ b/include/ui/kbd-state.h > @@ -9,9 +9,7 @@ > > #include "qapi/qapi-types-ui.h" > > -typedef enum QKbdModifier QKbdModifier; > - > -enum QKbdModifier { > +typedef enum QKbdModifier { > QKBD_MOD_NONE = 0, > > QKBD_MOD_SHIFT, > @@ -23,7 +21,7 @@ enum QKbdModifier { > QKBD_MOD_CAPSLOCK, > > QKBD_MOD__MAX > -}; > +} QKbdModifier; > > typedef struct QKbdState QKbdState; > > diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h > index 5c5e808a500..67fd0da9fd2 100644 > --- a/pc-bios/s390-ccw/virtio.h > +++ b/pc-bios/s390-ccw/virtio.h > @@ -173,13 +173,12 @@ struct VirtioBlkConfig { > } __attribute__((packed)); > typedef struct VirtioBlkConfig VirtioBlkConfig; > > -enum guessed_disk_nature_type { > +typedef enum guessed_disk_nature_type { > VIRTIO_GDN_NONE = 0, > VIRTIO_GDN_DASD = 1, > VIRTIO_GDN_CDROM = 2, > VIRTIO_GDN_SCSI = 3, > -}; > -typedef enum guessed_disk_nature_type VirtioGDN; > +} VirtioGDN; > > VirtioGDN virtio_guessed_disk_nature(void); > void virtio_assume_eckd(void); > diff --git a/tests/qtest/libqos/qgraph_internal.h b/tests/qtest/libqos/qgraph_internal.h > index 87fab1f9f0d..e2e740a59db 100644 > --- a/tests/qtest/libqos/qgraph_internal.h > +++ b/tests/qtest/libqos/qgraph_internal.h > @@ -27,8 +27,6 @@ > #include "qgraph.h" > > typedef struct QOSGraphMachine QOSGraphMachine; > -typedef enum QOSEdgeType QOSEdgeType; > -typedef enum QOSNodeType QOSNodeType; > > /* callback called when the walk path algorithm found a > * valid path > @@ -36,19 +34,19 @@ typedef enum QOSNodeType QOSNodeType; > typedef void (*QOSTestCallback) (QOSGraphNode *path, int len); > > /* edge types*/ > -enum QOSEdgeType { > +typedef enum QOSEdgeType { > QEDGE_CONTAINS, > QEDGE_PRODUCES, > QEDGE_CONSUMED_BY > -}; > +} QOSEdgeType; > > /* node types*/ > -enum QOSNodeType { > +typedef enum QOSNodeType { > QNODE_MACHINE, > QNODE_DRIVER, > QNODE_INTERFACE, > QNODE_TEST > -}; > +} QOSNodeType; > > /* Graph Node */ > struct QOSGraphNode { > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 5cbfba0a86d..5b6f022428c 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -1688,15 +1688,14 @@ HumanReadableText *qmp_x_query_roms(Error **errp) > return human_readable_text_from_str(buf); > } > > -typedef enum HexRecord HexRecord; > -enum HexRecord { > +typedef enum HexRecord { > DATA_RECORD = 0, > EOF_RECORD, > EXT_SEG_ADDR_RECORD, > START_SEG_ADDR_RECORD, > EXT_LINEAR_ADDR_RECORD, > START_LINEAR_ADDR_RECORD, > -}; > +} HexRecord; > > /* Each record contains a 16-bit address which is combined with the upper 16 > * bits of the implicit "next address" to form a 32-bit address. > diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c > index a24e95f0a73..1ab2375625c 100644 > --- a/hw/display/xlnx_dp.c > +++ b/hw/display/xlnx_dp.c > @@ -212,7 +212,7 @@ > #define DP_GRAPHIC_DMA_CHANNEL (3) > #define DP_VIDEO_DMA_CHANNEL (0) > > -enum DPGraphicFmt { > +typedef enum DPGraphicFmt { > DP_GRAPHIC_RGBA8888 = 0 << 8, > DP_GRAPHIC_ABGR8888 = 1 << 8, > DP_GRAPHIC_RGB888 = 2 << 8, > @@ -225,9 +225,9 @@ enum DPGraphicFmt { > DP_GRAPHIC_2BPP = 9 << 8, > DP_GRAPHIC_1BPP = 10 << 8, > DP_GRAPHIC_MASK = 0xF << 8 > -}; > +} DPGraphicFmt; > > -enum DPVideoFmt { > +typedef enum DPVideoFmt { > DP_NL_VID_CB_Y0_CR_Y1 = 0, > DP_NL_VID_CR_Y0_CB_Y1 = 1, > DP_NL_VID_Y0_CR_Y1_CB = 2, > @@ -254,10 +254,7 @@ enum DPVideoFmt { > DP_NL_VID_YV16CL_420_10BPC = 23, > DP_NL_VID_YV16CL2_420_10BPC = 24, > DP_NL_VID_FMT_MASK = 0x1F > -}; > - > -typedef enum DPGraphicFmt DPGraphicFmt; > -typedef enum DPVideoFmt DPVideoFmt; > +} DPVideoFmt; > > static const VMStateDescription vmstate_dp = { > .name = TYPE_XLNX_DP, > diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c > index 32f56036a4c..ef5488b3eca 100644 > --- a/hw/dma/xlnx_dpdma.c > +++ b/hw/dma/xlnx_dpdma.c > @@ -122,15 +122,15 @@ > > #define DPDMA_FRAG_MAX_SZ (4096) > > -enum DPDMABurstType { > +typedef enum DPDMABurstType { > DPDMA_INCR = 0, > DPDMA_FIXED = 1 > -}; > +} DPDMABurstType; > > -enum DPDMAMode { > +typedef enum DPDMAMode { > DPDMA_CONTIGOUS = 0, > DPDMA_FRAGMENTED = 1 > -}; > +} DPDMAMode; > > struct DPDMADescriptor { > uint32_t control; > @@ -153,8 +153,6 @@ struct DPDMADescriptor { > uint32_t crc; > }; > > -typedef enum DPDMABurstType DPDMABurstType; > -typedef enum DPDMAMode DPDMAMode; > typedef struct DPDMADescriptor DPDMADescriptor; > > static bool xlnx_dpdma_desc_is_last(DPDMADescriptor *desc) > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c > index 3d1a28b4191..66b1b931a31 100644 > --- a/qapi/opts-visitor.c > +++ b/qapi/opts-visitor.c > @@ -20,8 +20,7 @@ > #include "qapi/visitor-impl.h" > > > -enum ListMode > -{ > +typedef enum ListMode { > LM_NONE, /* not traversing a list of repeated options */ > > LM_IN_PROGRESS, /* > @@ -58,9 +57,7 @@ enum ListMode > * No more QemuOpt instance in the list. > * The traversal has been completed. > */ > -}; > - > -typedef enum ListMode ListMode; > +} ListMode; > > struct OptsVisitor > { > diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c > index 5115536b153..5d175773356 100644 > --- a/qapi/string-output-visitor.c > +++ b/qapi/string-output-visitor.c > @@ -17,7 +17,7 @@ > #include <math.h> > #include "qemu/range.h" > > -enum ListMode { > +typedef enum ListMode { > LM_NONE, /* not traversing a list of repeated options */ > LM_STARTED, /* next_list() ready to be called */ > > @@ -48,9 +48,7 @@ enum ListMode { > LM_UNSIGNED_INTERVAL,/* Same as above, only for an unsigned interval. */ > > LM_END, /* next_list() called, about to see last element. */ > -}; > - > -typedef enum ListMode ListMode; > +} ListMode; > > struct StringOutputVisitor > { > diff --git a/tests/unit/check-qom-proplist.c b/tests/unit/check-qom-proplist.c > index ee3c6fb32b1..6f3dbc4407a 100644 > --- a/tests/unit/check-qom-proplist.c > +++ b/tests/unit/check-qom-proplist.c > @@ -40,15 +40,13 @@ typedef struct DummyObjectClass DummyObjectClass; > DECLARE_INSTANCE_CHECKER(DummyObject, DUMMY_OBJECT, > TYPE_DUMMY) > > -typedef enum DummyAnimal DummyAnimal; > - > -enum DummyAnimal { > +typedef enum DummyAnimal { > DUMMY_FROG, > DUMMY_ALLIGATOR, > DUMMY_PLATYPUS, > > DUMMY_LAST, > -}; > +} DummyAnimal; > > const QEnumLookup dummy_animal_map = { > .array = (const char *const[]) { -- Alex Bennée Virtualisation Tech Lead @ Linaro ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-02-02 11:50 ` Alex Bennée @ 2026-02-02 12:05 ` Philippe Mathieu-Daudé 2026-02-02 13:04 ` Alex Bennée 2026-02-02 13:22 ` Daniel P. Berrangé 0 siblings, 2 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2026-02-02 12:05 UTC (permalink / raw) To: Alex Bennée Cc: qemu-devel, jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake On 2/2/26 12:50, Alex Bennée wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> 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 <rkir@google.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> --- >> 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. 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. Regards, Phil. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-02-02 12:05 ` Philippe Mathieu-Daudé @ 2026-02-02 13:04 ` Alex Bennée 2026-02-02 13:22 ` Daniel P. Berrangé 1 sibling, 0 replies; 8+ messages in thread From: Alex Bennée @ 2026-02-02 13:04 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: qemu-devel, jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 2/2/26 12:50, Alex Bennée wrote: >> Philippe Mathieu-Daudé <philmd@linaro.org> 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 <rkir@google.com> >>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>> --- >>> 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] bulk: Stop forward-declaring enum typedefs 2026-02-02 12:05 ` Philippe Mathieu-Daudé 2026-02-02 13:04 ` Alex Bennée @ 2026-02-02 13:22 ` Daniel P. Berrangé 1 sibling, 0 replies; 8+ messages in thread From: Daniel P. Berrangé @ 2026-02-02 13:22 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Alex Bennée, qemu-devel, jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake On Mon, Feb 02, 2026 at 01:05:33PM +0100, Philippe Mathieu-Daudé wrote: > On 2/2/26 12:50, Alex Bennée wrote: > > Philippe Mathieu-Daudé <philmd@linaro.org> 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 <rkir@google.com> > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > > Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > > > --- > > > 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 This kind of personally directed language is unhelpful. > to open the project to more contributors, even if changes > required are only cosmetics, and less invasive than Rust ones. The project as a whole has debated C++ multiple times over the years, and there were of course different opinions. Eventually the consensus for making QEMU sustainable & interesting to new contributors over the long term was to focus on Rust. Consensus does not imply unanimous, so there are doubtless still people, (whether upstream or dealing with forks) whom have a preference for C++. Given the decision to focus on Rust, however, work that facilitates or encourages use C++ at is sending people down a dead end path, and is a distraction for upstream reviewers/maintainers. It may look like an "quick win" but IMHO it is unhelpful (perhaps even harmful) over the longer time. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] docs/devel: Mention enum typedefs forward-declaration is not allowed 2026-01-30 21:17 [PATCH v2 0/2] bulk: Stop forward-declaring enum typedefs Philippe Mathieu-Daudé 2026-01-30 21:17 ` [PATCH v2 1/2] " Philippe Mathieu-Daudé @ 2026-01-30 21:17 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 8+ messages in thread From: Philippe Mathieu-Daudé @ 2026-01-30 21:17 UTC (permalink / raw) To: qemu-devel Cc: jpcottin, Markus Armbruster, Roman Kiryanov, Will Hollins, Paolo Bonzini, Peter Maydell, jansene, Eric Blake Do not allow enum typedef forward-declaration to comply with C99 standard chapter §6.7.2.2 point 4: 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. Update checkpatch.pl to catch further additions. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> --- docs/devel/style.rst | 11 +++++++++++ scripts/checkpatch.pl | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/docs/devel/style.rst b/docs/devel/style.rst index 12e509d10de..5ab5c21447d 100644 --- a/docs/devel/style.rst +++ b/docs/devel/style.rst @@ -416,6 +416,17 @@ definitions instead of typedefs in headers and function prototypes; this avoids problems with duplicated typedefs and reduces the need to include headers from other headers. +Enumeration (enum) type can not be forward declared as typedef, because +C compilers should be able to know the size of enums before hand. Simply +define the typedef along with the enum: + +.. code-block:: c + + typedef enum MyEnum { + FOO, + BAR, + } MyEnum; + Bitfields --------- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3a9557417f7..119b2a6c002 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2414,6 +2414,11 @@ sub process { ERROR("missing space after $1 definition\n" . $herecurr); } +# forward declared enum typedef + if ($line =~ /^.\s*typedef\s+enum(?:\s+$Ident)?(?:\s+$Ident)?;/) { + ERROR("forward declared enum typedef\n" . $herecurr); + } + # check for spacing round square brackets; allowed: # 1. with a type on the left -- int [] a; # 2. at the beginning of a line for slice initialisers -- [0...10] = 5, -- 2.52.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-02-02 13:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-30 21:17 [PATCH v2 0/2] bulk: Stop forward-declaring enum typedefs Philippe Mathieu-Daudé 2026-01-30 21:17 ` [PATCH v2 1/2] " Philippe Mathieu-Daudé 2026-02-02 11:19 ` Daniel P. Berrangé 2026-02-02 11:50 ` Alex Bennée 2026-02-02 12:05 ` Philippe Mathieu-Daudé 2026-02-02 13:04 ` Alex Bennée 2026-02-02 13:22 ` Daniel P. Berrangé 2026-01-30 21:17 ` [PATCH v2 2/2] docs/devel: Mention enum typedefs forward-declaration is not allowed Philippe Mathieu-Daudé
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.