Re: [PATCH 2/2] misc: Replace zero-length arrays with flexible array member (manual)

2020-03-04 Thread Philippe Mathieu-Daudé

On 3/4/20 1:58 AM, 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):

   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 
Signed-off-by: Philippe Mathieu-Daudé 
---
  docs/interop/vhost-user.rst   | 4 ++--
  block/qed.h   | 2 +-
  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 +-
  11 files changed, 16 insertions(+), 16 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/block/qed.h b/block/qed.h
index 42c115d822..87428ba00e 100644
--- a/block/qed.h
+++ b/block/qed.h
@@ -103,7 +103,7 @@ typedef struct {
  } QEMU_PACKED QEDHeader;
  
  typedef struct {

-uint64_t offsets[0];/* in bytes */
+uint64_t offsets[]; /* in bytes */


Apparently this one is incorrect, it triggers:

GCC:
block/qed.h:106:14: error: flexible array member in otherwise empty struct

Clang:
block/qed.h:106:14: error: flexible array member 'offsets' not allowed 
in otherwise empty struct



  } QEDTable;
  
  /* The L2 cache is a simple write-through cache for L2 structures */

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 {

  Md

Re: [PATCH 2/2] misc: Replace zero-length arrays with flexible array member (manual)

2020-03-04 Thread David Hildenbrand
On 04.03.20 01:58, 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):
> 
>   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 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  docs/interop/vhost-user.rst   | 4 ++--
>  block/qed.h   | 2 +-
>  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 +-
>  11 files changed, 16 insertions(+), 16 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/block/qed.h b/block/qed.h
> index 42c115d822..87428ba00e 100644
> --- a/block/qed.h
> +++ b/block/qed.h
> @@ -103,7 +103,7 @@ typedef struct {
>  } QEMU_PACKED QEDHeader;
>  
>  typedef struct {
> -uint64_t offsets[0];/* in bytes */
> +uint64_t offsets[]; /* in bytes */
>  } QEDTable;
>  
>  /* The L2 cache is a simple write-through cache for L2 structures */
> 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];
> +M