[PATCH v1] kdd: remove zero-length arrays

Olaf Hering posted 1 patch 3 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200608203849.18341-1-olaf@aepfle.de
Maintainers: Tim Deegan <tim@xen.org>, Wei Liu <wl@xen.org>, Ian Jackson <ian.jackson@eu.citrix.com>
tools/debugger/kdd/kdd.c | 10 +++++-----
tools/debugger/kdd/kdd.h |  3 +--
2 files changed, 6 insertions(+), 7 deletions(-)
[PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Struct 'kdd_hdr' already has a member named 'payload[]' to easily
refer to the data after the header.  Remove the payload member from
'kdd_pkt' and always use 'kdd_hdr' to fix the following compile error:

kdd.c: In function 'kdd_tx':
kdd.c:746:30: error: array subscript 65534 is outside the bounds of an interior zero-length array 'uint8_t[0]' {aka 'unsigned char[]'} [-Werror=zero-length-bounds]
  746 |         sum += s->txp.payload[i];
      |                ~~~~~~~~~~~~~~^~~
In file included from kdd.c:53:
kdd.h:326:17: note: while referencing 'payload'
  326 |         uint8_t payload[0];
      |                 ^~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 tools/debugger/kdd/kdd.c | 10 +++++-----
 tools/debugger/kdd/kdd.h |  3 +--
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 3ebda9b12c..4c6b39c22b 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -249,7 +249,7 @@ static void kdd_log_pkt(kdd_state *s, char *name, kdd_pkt *p)
 
     /* Re-check the checksum */
     for (i = 0; i < p->h.len; i++)
-        sum += p->payload[i];
+        sum += p->h.payload[i];
 
     fprintf(f, "\n"
             "%s: %s type 0x%4.4"PRIx16" len 0x%4.4"PRIx16
@@ -267,8 +267,8 @@ static void kdd_log_pkt(kdd_state *s, char *name, kdd_pkt *p)
             fprintf(f, "%8.8x ", i);
         } else if (i % 8 == 0)
             fprintf(f, " ");
-        fprintf(f, " %2.2x", p->payload[i]);
-        ascii[i % 16] = (isprint(((int)p->payload[i])) ? p->payload[i] : 0x2e);
+        fprintf(f, " %2.2x", p->h.payload[i]);
+        ascii[i % 16] = (isprint(((int)p->h.payload[i])) ? p->h.payload[i] : 0x2e);
         if (i % 16 == 15)
             fprintf(f, "  |%s|\n", ascii);
     }
@@ -743,7 +743,7 @@ static void kdd_tx(kdd_state *s)
 
     /* Fix up the checksum before we send */
     for (i = 0; i < s->txp.h.len; i++)
-        sum += s->txp.payload[i];
+        sum += s->txp.h.payload[i];
     s->txp.h.sum = sum;
 
     kdd_log_pkt(s, "TX", &s->txp);
@@ -1127,7 +1127,7 @@ static void kdd_handle_pkt(kdd_state *s, kdd_pkt *p)
 
     /* Simple checksum: add all the bytes */
     for (i = 0; i < p->h.len; i++)
-        sum += p->payload[i];
+        sum += p->h.payload[i];
     if (p->h.sum != sum) {
         kdd_send_ack(s, p->h.id, KDD_ACK_BAD);
         return;
diff --git a/tools/debugger/kdd/kdd.h b/tools/debugger/kdd/kdd.h
index bfb00ba5c5..57525d36c6 100644
--- a/tools/debugger/kdd/kdd.h
+++ b/tools/debugger/kdd/kdd.h
@@ -68,7 +68,7 @@ typedef struct {
     uint16_t len;     /* Payload length, excl. header and trailing byte */
     uint32_t id;      /* Echoed in responses */
     uint32_t sum;     /* Unsigned sum of all payload bytes */
-    uint8_t payload[0];
+    uint8_t payload[];
 } PACKED kdd_hdr;
 
 #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
@@ -323,7 +323,6 @@ typedef struct {
         kdd_msg msg;
         kdd_reg reg;
         kdd_stc stc;
-        uint8_t payload[0];
     };
 } PACKED kdd_pkt;
 

RE: [PATCH v1] kdd: remove zero-length arrays
Posted by Paul Durrant 3 years, 9 months ago
> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Olaf Hering
> Sent: 08 June 2020 21:39
> To: xen-devel@lists.xenproject.org
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>; Olaf Hering <olaf@aepfle.de>; Tim Deegan <tim@xen.org>;
> Wei Liu <wl@xen.org>
> Subject: [PATCH v1] kdd: remove zero-length arrays
> 
> Struct 'kdd_hdr' already has a member named 'payload[]' to easily
> refer to the data after the header.  Remove the payload member from
> 'kdd_pkt' and always use 'kdd_hdr' to fix the following compile error:

Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []? In fact I can't see any use of the payload
field in kdd_hdr so it looks like that is the one that ought to be dropped.

  Paul

> 
> kdd.c: In function 'kdd_tx':
> kdd.c:746:30: error: array subscript 65534 is outside the bounds of an interior zero-length array
> 'uint8_t[0]' {aka 'unsigned char[]'} [-Werror=zero-length-bounds]
>   746 |         sum += s->txp.payload[i];
>       |                ~~~~~~~~~~~~~~^~~
> In file included from kdd.c:53:
> kdd.h:326:17: note: while referencing 'payload'
>   326 |         uint8_t payload[0];
>       |                 ^~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
> ---
>  tools/debugger/kdd/kdd.c | 10 +++++-----
>  tools/debugger/kdd/kdd.h |  3 +--
>  2 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
> index 3ebda9b12c..4c6b39c22b 100644
> --- a/tools/debugger/kdd/kdd.c
> +++ b/tools/debugger/kdd/kdd.c
> @@ -249,7 +249,7 @@ static void kdd_log_pkt(kdd_state *s, char *name, kdd_pkt *p)
> 
>      /* Re-check the checksum */
>      for (i = 0; i < p->h.len; i++)
> -        sum += p->payload[i];
> +        sum += p->h.payload[i];
> 
>      fprintf(f, "\n"
>              "%s: %s type 0x%4.4"PRIx16" len 0x%4.4"PRIx16
> @@ -267,8 +267,8 @@ static void kdd_log_pkt(kdd_state *s, char *name, kdd_pkt *p)
>              fprintf(f, "%8.8x ", i);
>          } else if (i % 8 == 0)
>              fprintf(f, " ");
> -        fprintf(f, " %2.2x", p->payload[i]);
> -        ascii[i % 16] = (isprint(((int)p->payload[i])) ? p->payload[i] : 0x2e);
> +        fprintf(f, " %2.2x", p->h.payload[i]);
> +        ascii[i % 16] = (isprint(((int)p->h.payload[i])) ? p->h.payload[i] : 0x2e);
>          if (i % 16 == 15)
>              fprintf(f, "  |%s|\n", ascii);
>      }
> @@ -743,7 +743,7 @@ static void kdd_tx(kdd_state *s)
> 
>      /* Fix up the checksum before we send */
>      for (i = 0; i < s->txp.h.len; i++)
> -        sum += s->txp.payload[i];
> +        sum += s->txp.h.payload[i];
>      s->txp.h.sum = sum;
> 
>      kdd_log_pkt(s, "TX", &s->txp);
> @@ -1127,7 +1127,7 @@ static void kdd_handle_pkt(kdd_state *s, kdd_pkt *p)
> 
>      /* Simple checksum: add all the bytes */
>      for (i = 0; i < p->h.len; i++)
> -        sum += p->payload[i];
> +        sum += p->h.payload[i];
>      if (p->h.sum != sum) {
>          kdd_send_ack(s, p->h.id, KDD_ACK_BAD);
>          return;
> diff --git a/tools/debugger/kdd/kdd.h b/tools/debugger/kdd/kdd.h
> index bfb00ba5c5..57525d36c6 100644
> --- a/tools/debugger/kdd/kdd.h
> +++ b/tools/debugger/kdd/kdd.h
> @@ -68,7 +68,7 @@ typedef struct {
>      uint16_t len;     /* Payload length, excl. header and trailing byte */
>      uint32_t id;      /* Echoed in responses */
>      uint32_t sum;     /* Unsigned sum of all payload bytes */
> -    uint8_t payload[0];
> +    uint8_t payload[];
>  } PACKED kdd_hdr;
> 
>  #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
> @@ -323,7 +323,6 @@ typedef struct {
>          kdd_msg msg;
>          kdd_reg reg;
>          kdd_stc stc;
> -        uint8_t payload[0];
>      };
>  } PACKED kdd_pkt;
> 



Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Am Tue, 9 Jun 2020 09:55:52 +0100
schrieb Paul Durrant <xadimgnik@gmail.com>:

> Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []?

AFAIR this lead to compile errors.

Olaf
RE: [PATCH v1] kdd: remove zero-length arrays
Posted by Paul Durrant 3 years, 9 months ago
> -----Original Message-----
> From: Olaf Hering <olaf@aepfle.de>
> Sent: 09 June 2020 10:00
> To: Paul Durrant <xadimgnik@gmail.com>
> Cc: paul@xen.org; xen-devel@lists.xenproject.org; 'Ian Jackson' <ian.jackson@eu.citrix.com>; 'Tim
> Deegan' <tim@xen.org>; 'Wei Liu' <wl@xen.org>
> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> 
> Am Tue, 9 Jun 2020 09:55:52 +0100
> schrieb Paul Durrant <xadimgnik@gmail.com>:
> 
> > Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []?
> 
> AFAIR this lead to compile errors.
> 

OOI which compiler (might be worth mentioning in the commit comment too, for reference)? I'm not seeing a problem.

  Paul

> Olaf


Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Jürgen Groß 3 years, 9 months ago
On 09.06.20 11:04, Paul Durrant wrote:
>> -----Original Message-----
>> From: Olaf Hering <olaf@aepfle.de>
>> Sent: 09 June 2020 10:00
>> To: Paul Durrant <xadimgnik@gmail.com>
>> Cc: paul@xen.org; xen-devel@lists.xenproject.org; 'Ian Jackson' <ian.jackson@eu.citrix.com>; 'Tim
>> Deegan' <tim@xen.org>; 'Wei Liu' <wl@xen.org>
>> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
>>
>> Am Tue, 9 Jun 2020 09:55:52 +0100
>> schrieb Paul Durrant <xadimgnik@gmail.com>:
>>
>>> Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []?
>>
>> AFAIR this lead to compile errors.
>>
> 
> OOI which compiler (might be worth mentioning in the commit comment too, for reference)? I'm not seeing a problem.

We don't use array[] in public headers, but AFAIK using them internally
is fine.


Juergen


RE: [PATCH v1] kdd: remove zero-length arrays
Posted by Paul Durrant 3 years, 9 months ago
> -----Original Message-----
> From: Jürgen Groß <jgross@suse.com>
> Sent: 09 June 2020 10:06
> To: paul@xen.org; 'Olaf Hering' <olaf@aepfle.de>; 'Paul Durrant' <xadimgnik@gmail.com>
> Cc: xen-devel@lists.xenproject.org; 'Ian Jackson' <ian.jackson@eu.citrix.com>; 'Wei Liu' <wl@xen.org>;
> 'Tim Deegan' <tim@xen.org>
> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> 
> On 09.06.20 11:04, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Olaf Hering <olaf@aepfle.de>
> >> Sent: 09 June 2020 10:00
> >> To: Paul Durrant <xadimgnik@gmail.com>
> >> Cc: paul@xen.org; xen-devel@lists.xenproject.org; 'Ian Jackson' <ian.jackson@eu.citrix.com>; 'Tim
> >> Deegan' <tim@xen.org>; 'Wei Liu' <wl@xen.org>
> >> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> >>
> >> Am Tue, 9 Jun 2020 09:55:52 +0100
> >> schrieb Paul Durrant <xadimgnik@gmail.com>:
> >>
> >>> Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []?
> >>
> >> AFAIR this lead to compile errors.
> >>
> >
> > OOI which compiler (might be worth mentioning in the commit comment too, for reference)? I'm not
> seeing a problem.
> 
> We don't use array[] in public headers, but AFAIK using them internally
> is fine.
> 

Yeah, we have XEN_FLEX_ARRAY_DIM for public headers.

  Paul

> 
> Juergen



Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Am Tue, 9 Jun 2020 10:04:30 +0100
schrieb Paul Durrant <xadimgnik@gmail.com>:

> OOI which compiler (might be worth mentioning in the commit comment too, for reference)? I'm not seeing a problem.

This is from gcc10. I think the build automation for Tumbleweed will show this error, unless the Tumbleweed image is older than a week.

Olaf
Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Tim Deegan 3 years, 9 months ago
Hi,

At 09:55 +0100 on 09 Jun (1591696552), Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Olaf Hering
> > Sent: 08 June 2020 21:39
> > To: xen-devel@lists.xenproject.org
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>; Olaf Hering <olaf@aepfle.de>; Tim Deegan <tim@xen.org>;
> > Wei Liu <wl@xen.org>
> > Subject: [PATCH v1] kdd: remove zero-length arrays
> > 
> > Struct 'kdd_hdr' already has a member named 'payload[]' to easily
> > refer to the data after the header.  Remove the payload member from
> > 'kdd_pkt' and always use 'kdd_hdr' to fix the following compile error:
> 
> Is it not sufficient to just change the declaration of payload in kdd_pkt from [0] to []? In fact I can't see any use of the payload
> field in kdd_hdr so it looks like that is the one that ought to be dropped.

Yes, if one of these has to go, it should be the one in the header,
since it's not used and the one in the packet is neatly in the union
with the other decriptions of the packet payload.

I'm not currently in a position to test patches, but might be later in
the week.  Olaf, can you try dropping the 'payload' field from the
header and replacing the payload[0] in pkt with payload[] ?

Cheers,

Tim.

Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Am Tue, 9 Jun 2020 13:15:49 +0100
schrieb Tim Deegan <tim@xen.org>:

> Olaf, can you try dropping the 'payload' field from the header and replacing the payload[0] in pkt with payload[] ?

In file included from kdd.c:53:
kdd.h:325:17: error: flexible array member in union
  325 |         uint8_t payload[];

--- orig/kdd.h  2020-06-08 17:40:05.000000000 +0000
+++ kdd.h       2020-06-09 13:20:36.724887538 +0000
@@ -68,7 +68,6 @@
     uint16_t len;     /* Payload length, excl. header and trailing byte */
     uint32_t id;      /* Echoed in responses */
     uint32_t sum;     /* Unsigned sum of all payload bytes */
-    uint8_t payload[0];
 } PACKED kdd_hdr;
 
 #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
@@ -323,7 +322,7 @@
         kdd_msg msg;
         kdd_reg reg;
         kdd_stc stc;
-        uint8_t payload[0];
+        uint8_t payload[];
     };
 } PACKED kdd_pkt;
 

Olaf
Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Tim Deegan 3 years, 9 months ago
At 15:22 +0200 on 09 Jun (1591716153), Olaf Hering wrote:
> Am Tue, 9 Jun 2020 13:15:49 +0100
> schrieb Tim Deegan <tim@xen.org>:
> 
> > Olaf, can you try dropping the 'payload' field from the header and replacing the payload[0] in pkt with payload[] ?
> 
> In file included from kdd.c:53:
> kdd.h:325:17: error: flexible array member in union
>   325 |         uint8_t payload[];

How tedious.  Well, the only place we actually allocate one of these
we already leave enough space for a max-size packet, so how about
this?

kdd: stop using [0] arrays to access packet contents.

GCC 10 is unhappy about this, and we already use 64k buffers
in the only places where packets are allocated, so move the
64k size into the packet definition.

Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Tim Deegan <tim@xen.org>
---
 tools/debugger/kdd/kdd.c | 4 ++--
 tools/debugger/kdd/kdd.h | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git tools/debugger/kdd/kdd.c tools/debugger/kdd/kdd.c
index 3ebda9b12c..a7d0976ea4 100644
--- tools/debugger/kdd/kdd.c
+++ tools/debugger/kdd/kdd.c
@@ -79,11 +79,11 @@ typedef struct {
 /* State of the debugger stub */
 typedef struct {
     union {
-        uint8_t txb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for tx */
+        uint8_t txb[sizeof (kdd_pkt)];           /* Marshalling area for tx */
         kdd_pkt txp;                 /* Also readable as a packet structure */
     };
     union {
-        uint8_t rxb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for rx */
+        uint8_t rxb[sizeof (kdd_pkt)];           /* Marshalling area for rx */
         kdd_pkt rxp;                 /* Also readable as a packet structure */
     };
     unsigned int cur;       /* Offset into rx where we'll put the next byte */
diff --git tools/debugger/kdd/kdd.h tools/debugger/kdd/kdd.h
index bfb00ba5c5..b9a17440df 100644
--- tools/debugger/kdd/kdd.h
+++ tools/debugger/kdd/kdd.h
@@ -68,7 +68,6 @@ typedef struct {
     uint16_t len;     /* Payload length, excl. header and trailing byte */
     uint32_t id;      /* Echoed in responses */
     uint32_t sum;     /* Unsigned sum of all payload bytes */
-    uint8_t payload[0];
 } PACKED kdd_hdr;
 
 #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
@@ -323,7 +322,7 @@ typedef struct {
         kdd_msg msg;
         kdd_reg reg;
         kdd_stc stc;
-        uint8_t payload[0];
+        uint8_t payload[65536];
     };
 } PACKED kdd_pkt;
 
-- 
2.26.2


Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Am Wed, 10 Jun 2020 20:16:57 +0100
schrieb Tim Deegan <tim@xen.org>:

> How tedious.

Indeed. This compiles for me as well:

--- orig/kdd.h  2020-06-08 17:40:05.000000000 +0000
+++ kdd.h       2020-06-11 19:00:44.234364040 +0000
@@ -68,7 +68,6 @@
     uint16_t len;     /* Payload length, excl. header and trailing byte */
     uint32_t id;      /* Echoed in responses */
     uint32_t sum;     /* Unsigned sum of all payload bytes */
-    uint8_t payload[0];
 } PACKED kdd_hdr;
 
 #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
@@ -323,7 +322,7 @@
         kdd_msg msg;
         kdd_reg reg;
         kdd_stc stc;
-        uint8_t payload[0];
+        uint8_t payload[65536];
     };
 } PACKED kdd_pkt;
 
--- orig/kdd.c  2020-06-08 17:40:05.000000000 +0000
+++ kdd.c       2020-06-11 19:08:36.775724640 +0000
@@ -79,11 +79,11 @@
 /* State of the debugger stub */
 typedef struct {
     union {
-        uint8_t txb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for tx */
+        uint8_t txb[sizeof (kdd_hdr) + 0xffff];   /* Marshalling area for tx */
         kdd_pkt txp;                 /* Also readable as a packet structure */
     };
     union {
-        uint8_t rxb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for rx */
+        uint8_t rxb[sizeof (kdd_hdr)];   /* Marshalling area for rx */
         kdd_pkt rxp;                 /* Also readable as a packet structure */
     };
     unsigned int cur;       /* Offset into rx where we'll put the next byte */

Olaf
Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Christopher Clark 3 years, 9 months ago
On Thu, Jun 11, 2020 at 12:12 PM Olaf Hering <olaf@aepfle.de> wrote:
>
> Am Wed, 10 Jun 2020 20:16:57 +0100
> schrieb Tim Deegan <tim@xen.org>:
>
> > How tedious.
>
> Indeed. This compiles for me as well:

just a nudge on this; it would be nice to get a patch into the tree
since the build failure affects master builds of Xen in OpenEmbedded
now.

Christopher

>
> --- orig/kdd.h  2020-06-08 17:40:05.000000000 +0000
> +++ kdd.h       2020-06-11 19:00:44.234364040 +0000
> @@ -68,7 +68,6 @@
>      uint16_t len;     /* Payload length, excl. header and trailing byte */
>      uint32_t id;      /* Echoed in responses */
>      uint32_t sum;     /* Unsigned sum of all payload bytes */
> -    uint8_t payload[0];
>  } PACKED kdd_hdr;
>
>  #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
> @@ -323,7 +322,7 @@
>          kdd_msg msg;
>          kdd_reg reg;
>          kdd_stc stc;
> -        uint8_t payload[0];
> +        uint8_t payload[65536];
>      };
>  } PACKED kdd_pkt;
>
> --- orig/kdd.c  2020-06-08 17:40:05.000000000 +0000
> +++ kdd.c       2020-06-11 19:08:36.775724640 +0000
> @@ -79,11 +79,11 @@
>  /* State of the debugger stub */
>  typedef struct {
>      union {
> -        uint8_t txb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for tx */
> +        uint8_t txb[sizeof (kdd_hdr) + 0xffff];   /* Marshalling area for tx */
>          kdd_pkt txp;                 /* Also readable as a packet structure */
>      };
>      union {
> -        uint8_t rxb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for rx */
> +        uint8_t rxb[sizeof (kdd_hdr)];   /* Marshalling area for rx */
>          kdd_pkt rxp;                 /* Also readable as a packet structure */
>      };
>      unsigned int cur;       /* Offset into rx where we'll put the next byte */
>
> Olaf

RE: [PATCH v1] kdd: remove zero-length arrays
Posted by Paul Durrant 3 years, 9 months ago
> -----Original Message-----
> From: Christopher Clark <christopher.w.clark@gmail.com>
> Sent: 16 June 2020 21:50
> To: Olaf Hering <olaf@aepfle.de>
> Cc: Tim Deegan <tim@xen.org>; xen-devel <xen-devel@lists.xenproject.org>; Ian Jackson
> <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; paul@xen.org
> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> 
> On Thu, Jun 11, 2020 at 12:12 PM Olaf Hering <olaf@aepfle.de> wrote:
> >
> > Am Wed, 10 Jun 2020 20:16:57 +0100
> > schrieb Tim Deegan <tim@xen.org>:
> >
> > > How tedious.
> >
> > Indeed. This compiles for me as well:
> 
> just a nudge on this; it would be nice to get a patch into the tree
> since the build failure affects master builds of Xen in OpenEmbedded
> now.
> 

I'd be happy to take a patch into 4.14 if someone can provide one with a suitable maintainer ack.

  Paul

> Christopher
> 
> >
> > --- orig/kdd.h  2020-06-08 17:40:05.000000000 +0000
> > +++ kdd.h       2020-06-11 19:00:44.234364040 +0000
> > @@ -68,7 +68,6 @@
> >      uint16_t len;     /* Payload length, excl. header and trailing byte */
> >      uint32_t id;      /* Echoed in responses */
> >      uint32_t sum;     /* Unsigned sum of all payload bytes */
> > -    uint8_t payload[0];
> >  } PACKED kdd_hdr;
> >
> >  #define KDD_PKT_CMD 0x0002      /* Debugger commands (and replies to them) */
> > @@ -323,7 +322,7 @@
> >          kdd_msg msg;
> >          kdd_reg reg;
> >          kdd_stc stc;
> > -        uint8_t payload[0];
> > +        uint8_t payload[65536];
> >      };
> >  } PACKED kdd_pkt;
> >
> > --- orig/kdd.c  2020-06-08 17:40:05.000000000 +0000
> > +++ kdd.c       2020-06-11 19:08:36.775724640 +0000
> > @@ -79,11 +79,11 @@
> >  /* State of the debugger stub */
> >  typedef struct {
> >      union {
> > -        uint8_t txb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for tx */
> > +        uint8_t txb[sizeof (kdd_hdr) + 0xffff];   /* Marshalling area for tx */
> >          kdd_pkt txp;                 /* Also readable as a packet structure */
> >      };
> >      union {
> > -        uint8_t rxb[sizeof (kdd_hdr) + 65536];   /* Marshalling area for rx */
> > +        uint8_t rxb[sizeof (kdd_hdr)];   /* Marshalling area for rx */
> >          kdd_pkt rxp;                 /* Also readable as a packet structure */
> >      };
> >      unsigned int cur;       /* Offset into rx where we'll put the next byte */
> >
> > Olaf


Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Wei Liu 3 years, 9 months ago
On Wed, Jun 17, 2020 at 09:21:22AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Christopher Clark <christopher.w.clark@gmail.com>
> > Sent: 16 June 2020 21:50
> > To: Olaf Hering <olaf@aepfle.de>
> > Cc: Tim Deegan <tim@xen.org>; xen-devel <xen-devel@lists.xenproject.org>; Ian Jackson
> > <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; paul@xen.org
> > Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> > 
> > On Thu, Jun 11, 2020 at 12:12 PM Olaf Hering <olaf@aepfle.de> wrote:
> > >
> > > Am Wed, 10 Jun 2020 20:16:57 +0100
> > > schrieb Tim Deegan <tim@xen.org>:
> > >
> > > > How tedious.
> > >
> > > Indeed. This compiles for me as well:
> > 
> > just a nudge on this; it would be nice to get a patch into the tree
> > since the build failure affects master builds of Xen in OpenEmbedded
> > now.
> > 
> 
> I'd be happy to take a patch into 4.14 if someone can provide one with a suitable maintainer ack.

Tim is the maintainer of KDD. :-)

I take it you're happy with me committing his patch then?

Wei.

RE: [PATCH v1] kdd: remove zero-length arrays
Posted by Paul Durrant 3 years, 9 months ago
> -----Original Message-----
> From: Wei Liu <wl@xen.org>
> Sent: 26 June 2020 11:27
> To: paul@xen.org
> Cc: 'Christopher Clark' <christopher.w.clark@gmail.com>; 'Olaf Hering' <olaf@aepfle.de>; 'Tim Deegan'
> <tim@xen.org>; 'xen-devel' <xen-devel@lists.xenproject.org>; 'Ian Jackson'
> <ian.jackson@eu.citrix.com>; 'Wei Liu' <wl@xen.org>
> Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> 
> On Wed, Jun 17, 2020 at 09:21:22AM +0100, Paul Durrant wrote:
> > > -----Original Message-----
> > > From: Christopher Clark <christopher.w.clark@gmail.com>
> > > Sent: 16 June 2020 21:50
> > > To: Olaf Hering <olaf@aepfle.de>
> > > Cc: Tim Deegan <tim@xen.org>; xen-devel <xen-devel@lists.xenproject.org>; Ian Jackson
> > > <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; paul@xen.org
> > > Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> > >
> > > On Thu, Jun 11, 2020 at 12:12 PM Olaf Hering <olaf@aepfle.de> wrote:
> > > >
> > > > Am Wed, 10 Jun 2020 20:16:57 +0100
> > > > schrieb Tim Deegan <tim@xen.org>:
> > > >
> > > > > How tedious.
> > > >
> > > > Indeed. This compiles for me as well:
> > >
> > > just a nudge on this; it would be nice to get a patch into the tree
> > > since the build failure affects master builds of Xen in OpenEmbedded
> > > now.
> > >
> >
> > I'd be happy to take a patch into 4.14 if someone can provide one with a suitable maintainer ack.
> 
> Tim is the maintainer of KDD. :-)
> 
> I take it you're happy with me committing his patch then?
> 

I'm happy, but ought it not have an ack from 'the rest' since Tim submitted the patch?

Release-acked-by: Paul Durrant <paul@xen.org>

> Wei.


Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Wei Liu 3 years, 9 months ago
On Fri, Jun 26, 2020 at 11:31:48AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Wei Liu <wl@xen.org>
> > Sent: 26 June 2020 11:27
> > To: paul@xen.org
> > Cc: 'Christopher Clark' <christopher.w.clark@gmail.com>; 'Olaf Hering' <olaf@aepfle.de>; 'Tim Deegan'
> > <tim@xen.org>; 'xen-devel' <xen-devel@lists.xenproject.org>; 'Ian Jackson'
> > <ian.jackson@eu.citrix.com>; 'Wei Liu' <wl@xen.org>
> > Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> > 
> > On Wed, Jun 17, 2020 at 09:21:22AM +0100, Paul Durrant wrote:
> > > > -----Original Message-----
> > > > From: Christopher Clark <christopher.w.clark@gmail.com>
> > > > Sent: 16 June 2020 21:50
> > > > To: Olaf Hering <olaf@aepfle.de>
> > > > Cc: Tim Deegan <tim@xen.org>; xen-devel <xen-devel@lists.xenproject.org>; Ian Jackson
> > > > <ian.jackson@eu.citrix.com>; Wei Liu <wl@xen.org>; paul@xen.org
> > > > Subject: Re: [PATCH v1] kdd: remove zero-length arrays
> > > >
> > > > On Thu, Jun 11, 2020 at 12:12 PM Olaf Hering <olaf@aepfle.de> wrote:
> > > > >
> > > > > Am Wed, 10 Jun 2020 20:16:57 +0100
> > > > > schrieb Tim Deegan <tim@xen.org>:
> > > > >
> > > > > > How tedious.
> > > > >
> > > > > Indeed. This compiles for me as well:
> > > >
> > > > just a nudge on this; it would be nice to get a patch into the tree
> > > > since the build failure affects master builds of Xen in OpenEmbedded
> > > > now.
> > > >
> > >
> > > I'd be happy to take a patch into 4.14 if someone can provide one with a suitable maintainer ack.
> > 
> > Tim is the maintainer of KDD. :-)
> > 
> > I take it you're happy with me committing his patch then?
> > 
> 
> I'm happy, but ought it not have an ack from 'the rest' since Tim submitted the patch?

Alright. FWIW:

Acked-by: Wei Liu <wl@xen.org>

> 
> Release-acked-by: Paul Durrant <paul@xen.org>

Thanks.

Wei.

> 
> > Wei.
> 

Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Ian Jackson 3 years, 9 months ago
Olaf Hering writes ("Re: [PATCH v1] kdd: remove zero-length arrays"):
> Am Tue, 9 Jun 2020 13:15:49 +0100
> schrieb Tim Deegan <tim@xen.org>:
> 
> > Olaf, can you try dropping the 'payload' field from the header and replacing the payload[0] in pkt with payload[] ?
> 
> In file included from kdd.c:53:
> kdd.h:325:17: error: flexible array member in union
>   325 |         uint8_t payload[];
...
>          kdd_stc stc;
> -        uint8_t payload[0];
> +        uint8_t payload[];
>      };
>  } PACKED kdd_pkt;

Try

>          kdd_stc stc;
> -        uint8_t payload[0];
>      };
  +    uint8_t payload[];
>  } PACKED kdd_pkt;

?

(I haven't read the surrounding code...)

Ian.

Re: [PATCH v1] kdd: remove zero-length arrays
Posted by Olaf Hering 3 years, 9 months ago
Am Tue, 9 Jun 2020 14:26:46 +0100
schrieb Ian Jackson <ian.jackson@citrix.com>:

>   +    uint8_t payload[];

This compiles, but will access memory behind the union{};, which is most likely not what the intention is.

Olaf