| Message ID | 20200304153816.23636-1-philmd@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | misc: Replace zero-length arrays with flexible array member | expand |
On 3/4/20 10:38 AM, Philippe Mathieu-Daudé wrote: > v2: > - do not modify qed.h (structure with single member) > - based on hw/scsi/spapr_vscsi fix series: > https://mid.mail-archive.com/20200304153311.22959-1-philmd@redhat.com > > This is a tree-wide cleanup inspired by a Linux kernel commit > (from Gustavo A. R. Silva). > > --v-- description start --v-- > > The current codebase makes use of the zero-length array language > extension to the C90 standard, but the preferred mechanism to > declare variable-length types such as these ones is a flexible > array member [1], introduced in C99: > > struct foo { > int stuff; > struct boo array[]; > }; > > By making use of the mechanism above, we will get a compiler > warning in case the flexible array does not occur last in the > structure, which will help us prevent some kind of undefined > behavior bugs from being unadvertenly introduced [2] to the > Linux codebase from now on. > > --^-- description end --^-- > > Do the similar housekeeping in the QEMU codebase (which uses > C99 since commit 7be41675f7cb). > > The first patch is done with the help of a coccinelle semantic > patch. However Coccinelle does not recognize: > > struct foo { > int stuff; > struct boo array[]; > } QEMU_PACKED; > > but does recognize: > > struct QEMU_PACKED foo { > int stuff; > struct boo array[]; > }; > > I'm not sure why, neither it is worth refactoring all QEMU > structures to use the attributes before the structure name, > so I did the 2nd patch manually. > > Anyway this is annoying, because many structures are not handled > by coccinelle. Maybe this needs to be reported to upstream > coccinelle? > > I used spatch 1.0.8 with: > > -I include --include-headers \ > --macro-file scripts/cocci-macro-file.h \ > --keep-comments --indent 4 > > Regards, > > Phil. > > Based-on: <20200304153311.22959-1-philmd@redhat.com> > Supersedes: <20200304005105.27454-1-philmd@redhat.com> > > Philippe Mathieu-Daudé (2): > misc: Replace zero-length arrays with flexible array member > (automatic) > misc: Replace zero-length arrays with flexible array member (manual) > > docs/interop/vhost-user.rst | 4 ++-- > bsd-user/qemu.h | 2 +- > contrib/libvhost-user/libvhost-user.h | 2 +- > hw/m68k/bootinfo.h | 2 +- > hw/scsi/srp.h | 6 +++--- > hw/xen/xen_pt.h | 2 +- > include/hw/acpi/acpi-defs.h | 16 ++++++++-------- > include/hw/arm/smmu-common.h | 2 +- > include/hw/boards.h | 2 +- > include/hw/i386/intel_iommu.h | 3 ++- > include/hw/s390x/event-facility.h | 2 +- > include/hw/s390x/sclp.h | 8 ++++---- > include/hw/virtio/virtio-iommu.h | 2 +- > include/sysemu/cryptodev.h | 2 +- > include/tcg/tcg.h | 2 +- > pc-bios/s390-ccw/bootmap.h | 2 +- > pc-bios/s390-ccw/sclp.h | 2 +- > tests/qtest/libqos/ahci.h | 2 +- > block/linux-aio.c | 2 +- > block/vmdk.c | 2 +- > hw/acpi/nvdimm.c | 6 +++--- > hw/char/sclpconsole-lm.c | 2 +- > hw/char/sclpconsole.c | 2 +- > hw/dma/soc_dma.c | 2 +- > hw/i386/x86.c | 2 +- > hw/misc/omap_l4.c | 2 +- > hw/nvram/eeprom93xx.c | 2 +- > hw/rdma/vmw/pvrdma_qp_ops.c | 4 ++-- > hw/s390x/virtio-ccw.c | 2 +- > hw/usb/dev-network.c | 2 +- > hw/usb/dev-smartcard-reader.c | 4 ++-- > hw/virtio/virtio.c | 4 ++-- > net/queue.c | 2 +- > target/s390x/ioinst.c | 2 +- > 34 files changed, 53 insertions(+), 52 deletions(-) > I'll admit I did not manually verify ALL of this, but instead trust that: 1. The conversion is correct, and this is a desirable change to make. 2. Sample conversions I looked at appear correct. 3. It builds. 4. It passes tests. So: Acked-by: John Snow <jsnow@redhat.com>
v2: - do not modify qed.h (structure with single member) - based on hw/scsi/spapr_vscsi fix series: https://mid.mail-archive.com/20200304153311.22959-1-philmd@redhat.com This is a tree-wide cleanup inspired by a Linux kernel commit (from Gustavo A. R. Silva). --v-- description start --v-- The current codebase makes use of the zero-length array language extension to the C90 standard, but the preferred mechanism to declare variable-length types such as these ones is a flexible array member [1], introduced in C99: struct foo { int stuff; struct boo array[]; }; By making use of the mechanism above, we will get a compiler warning in case the flexible array does not occur last in the structure, which will help us prevent some kind of undefined behavior bugs from being unadvertenly introduced [2] to the Linux codebase from now on. --^-- description end --^-- Do the similar housekeeping in the QEMU codebase (which uses C99 since commit 7be41675f7cb). The first patch is done with the help of a coccinelle semantic patch. However Coccinelle does not recognize: struct foo { int stuff; struct boo array[]; } QEMU_PACKED; but does recognize: struct QEMU_PACKED foo { int stuff; struct boo array[]; }; I'm not sure why, neither it is worth refactoring all QEMU structures to use the attributes before the structure name, so I did the 2nd patch manually. Anyway this is annoying, because many structures are not handled by coccinelle. Maybe this needs to be reported to upstream coccinelle? I used spatch 1.0.8 with: -I include --include-headers \ --macro-file scripts/cocci-macro-file.h \ --keep-comments --indent 4 Regards, Phil. Based-on: <20200304153311.22959-1-philmd@redhat.com> Supersedes: <20200304005105.27454-1-philmd@redhat.com> Philippe Mathieu-Daudé (2): misc: Replace zero-length arrays with flexible array member (automatic) misc: Replace zero-length arrays with flexible array member (manual) docs/interop/vhost-user.rst | 4 ++-- bsd-user/qemu.h | 2 +- contrib/libvhost-user/libvhost-user.h | 2 +- hw/m68k/bootinfo.h | 2 +- hw/scsi/srp.h | 6 +++--- hw/xen/xen_pt.h | 2 +- include/hw/acpi/acpi-defs.h | 16 ++++++++-------- include/hw/arm/smmu-common.h | 2 +- include/hw/boards.h | 2 +- include/hw/i386/intel_iommu.h | 3 ++- include/hw/s390x/event-facility.h | 2 +- include/hw/s390x/sclp.h | 8 ++++---- include/hw/virtio/virtio-iommu.h | 2 +- include/sysemu/cryptodev.h | 2 +- include/tcg/tcg.h | 2 +- pc-bios/s390-ccw/bootmap.h | 2 +- pc-bios/s390-ccw/sclp.h | 2 +- tests/qtest/libqos/ahci.h | 2 +- block/linux-aio.c | 2 +- block/vmdk.c | 2 +- hw/acpi/nvdimm.c | 6 +++--- hw/char/sclpconsole-lm.c | 2 +- hw/char/sclpconsole.c | 2 +- hw/dma/soc_dma.c | 2 +- hw/i386/x86.c | 2 +- hw/misc/omap_l4.c | 2 +- hw/nvram/eeprom93xx.c | 2 +- hw/rdma/vmw/pvrdma_qp_ops.c | 4 ++-- hw/s390x/virtio-ccw.c | 2 +- hw/usb/dev-network.c | 2 +- hw/usb/dev-smartcard-reader.c | 4 ++-- hw/virtio/virtio.c | 4 ++-- net/queue.c | 2 +- target/s390x/ioinst.c | 2 +- 34 files changed, 53 insertions(+), 52 deletions(-)