[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] misc: Replace zero-length arrays with flexible array
|
From: |
Michael S. Tsirkin |
|
Subject: |
Re: [PATCH v2 2/2] misc: Replace zero-length arrays with flexible array member (manual) |
|
Date: |
Sun, 8 Mar 2020 03:56:16 -0400 |
On Wed, Mar 04, 2020 at 04:38:16PM +0100, Philippe Mathieu-Daudé wrote:
> Description copied from Linux kernel commit from Gustavo A. R. Silva
> (see [3]):
>
> --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).
>
> All these instances of code were found with the help of the
> following command (then manual analysis, without modifying
> structures only having a single flexible array member, such
> QEDTable in block/qed.h):
>
> git grep -F '[0];'
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2]
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=76497732932f
> [3]
> https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/commit/?id=17642a2fbd2c1
>
> Inspired-by: Gustavo A. R. Silva <address@hidden>
> Reviewed-by: David Hildenbrand <address@hidden>
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
> ---
> v2: Do not modify block/qed.h:
>
> block/qed.h:106:14: error: flexible array member 'offsets' not allowed in
> otherwise empty struct
> uint64_t offsets[]; /* in bytes */
> ^
> ---
> docs/interop/vhost-user.rst | 4 ++--
> include/hw/acpi/acpi-defs.h | 4 ++--
> include/hw/boards.h | 2 +-
> include/hw/s390x/event-facility.h | 2 +-
> include/hw/s390x/sclp.h | 8 ++++----
> block/vmdk.c | 2 +-
> hw/char/sclpconsole-lm.c | 2 +-
> hw/char/sclpconsole.c | 2 +-
> hw/s390x/virtio-ccw.c | 2 +-
> target/s390x/ioinst.c | 2 +-
> 10 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index 401652397c..3b1b6602c7 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -568,7 +568,7 @@ For split virtqueue, queue region can be implemented as:
> uint16_t used_idx;
>
> /* Used to track the state of each descriptor in descriptor table */
> - DescStateSplit desc[0];
> + DescStateSplit desc[];
> } QueueRegionSplit;
>
> To track inflight I/O, the queue region should be processed as follows:
> @@ -690,7 +690,7 @@ For packed virtqueue, queue region can be implemented as:
> uint8_t padding[7];
>
> /* Used to track the state of each descriptor fetched from descriptor
> ring */
> - DescStatePacked desc[0];
> + DescStatePacked desc[];
> } QueueRegionPacked;
>
> To track inflight I/O, the queue region should be processed as follows:
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 19f7ba7b70..c13327fa78 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -152,7 +152,7 @@ typedef struct AcpiSerialPortConsoleRedirection
> */
> struct AcpiRsdtDescriptorRev1 {
> ACPI_TABLE_HEADER_DEF /* ACPI common table header */
> - uint32_t table_offset_entry[0]; /* Array of pointers to other */
> + uint32_t table_offset_entry[]; /* Array of pointers to other */
> /* ACPI tables */
> } QEMU_PACKED;
> typedef struct AcpiRsdtDescriptorRev1 AcpiRsdtDescriptorRev1;
> @@ -162,7 +162,7 @@ typedef struct AcpiRsdtDescriptorRev1
> AcpiRsdtDescriptorRev1;
> */
> struct AcpiXsdtDescriptorRev2 {
> ACPI_TABLE_HEADER_DEF /* ACPI common table header */
> - uint64_t table_offset_entry[0]; /* Array of pointers to other */
> + uint64_t table_offset_entry[]; /* Array of pointers to other */
> /* ACPI tables */
> } QEMU_PACKED;
> typedef struct AcpiXsdtDescriptorRev2 AcpiXsdtDescriptorRev2;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 9bc42dfb22..c96120d15f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -71,7 +71,7 @@ typedef struct CPUArchId {
> */
> typedef struct {
> int len;
> - CPUArchId cpus[0];
> + CPUArchId cpus[];
> } CPUArchIdList;
>
> /**
> diff --git a/include/hw/s390x/event-facility.h
> b/include/hw/s390x/event-facility.h
> index bdc32a3c09..700a610f33 100644
> --- a/include/hw/s390x/event-facility.h
> +++ b/include/hw/s390x/event-facility.h
> @@ -122,7 +122,7 @@ typedef struct MDBO {
>
> typedef struct MDB {
> MdbHeader header;
> - MDBO mdbo[0];
> + MDBO mdbo[];
> } QEMU_PACKED MDB;
>
> typedef struct SclpMsg {
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index c54413b78c..cd7b24359f 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -132,7 +132,7 @@ typedef struct ReadInfo {
> uint16_t highest_cpu;
> uint8_t _reserved5[124 - 122]; /* 122-123 */
> uint32_t hmfai;
> - struct CPUEntry entries[0];
> + struct CPUEntry entries[];
> } QEMU_PACKED ReadInfo;
>
> typedef struct ReadCpuInfo {
> @@ -142,7 +142,7 @@ typedef struct ReadCpuInfo {
> uint16_t nr_standby; /* 12-13 */
> uint16_t offset_standby; /* 14-15 */
> uint8_t reserved0[24-16]; /* 16-23 */
> - struct CPUEntry entries[0];
> + struct CPUEntry entries[];
> } QEMU_PACKED ReadCpuInfo;
>
> typedef struct ReadStorageElementInfo {
> @@ -151,7 +151,7 @@ typedef struct ReadStorageElementInfo {
> uint16_t assigned;
> uint16_t standby;
> uint8_t _reserved0[16 - 14]; /* 14-15 */
> - uint32_t entries[0];
> + uint32_t entries[];
> } QEMU_PACKED ReadStorageElementInfo;
>
> typedef struct AttachStorageElement {
> @@ -159,7 +159,7 @@ typedef struct AttachStorageElement {
> uint8_t _reserved0[10 - 8]; /* 8-9 */
> uint16_t assigned;
> uint8_t _reserved1[16 - 12]; /* 12-15 */
> - uint32_t entries[0];
> + uint32_t entries[];
> } QEMU_PACKED AttachStorageElement;
>
> typedef struct AssignStorage {
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 20e909d997..8466051bc9 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -187,7 +187,7 @@ typedef struct VmdkMetaData {
> typedef struct VmdkGrainMarker {
> uint64_t lba;
> uint32_t size;
> - uint8_t data[0];
> + uint8_t data[];
> } QEMU_PACKED VmdkGrainMarker;
>
> enum {
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index c420dc066e..2b5f37b6a2 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c
> @@ -31,7 +31,7 @@
> typedef struct OprtnsCommand {
> EventBufferHeader header;
> MDMSU message_unit;
> - char data[0];
> + char data[];
> } QEMU_PACKED OprtnsCommand;
>
> /* max size for line-mode data in 4K SCCB page */
> diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
> index 1fa124dab9..5c7664905e 100644
> --- a/hw/char/sclpconsole.c
> +++ b/hw/char/sclpconsole.c
> @@ -25,7 +25,7 @@
>
> typedef struct ASCIIConsoleData {
> EventBufferHeader ebh;
> - char data[0];
> + char data[];
> } QEMU_PACKED ASCIIConsoleData;
>
> /* max size for ASCII data in 4K SCCB page */
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 50cf95b781..64f928fc7d 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -193,7 +193,7 @@ typedef struct VirtioThinintInfo {
> typedef struct VirtioRevInfo {
> uint16_t revision;
> uint16_t length;
> - uint8_t data[0];
> + uint8_t data[];
> } QEMU_PACKED VirtioRevInfo;
>
> /* Specify where the virtqueues for the subchannel are in guest memory. */
> diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
> index c437a1d8c6..0e840cc579 100644
> --- a/target/s390x/ioinst.c
> +++ b/target/s390x/ioinst.c
> @@ -347,7 +347,7 @@ typedef struct ChscResp {
> uint16_t len;
> uint16_t code;
> uint32_t param;
> - char data[0];
> + char data[];
> } QEMU_PACKED ChscResp;
>
> #define CHSC_MIN_RESP_LEN 0x0008
> --
> 2.21.1