RE: [PATCH v1] kdd: remove zero-length arrays

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Olaf 
> Hering
> Sent: 08 June 2020 21:39
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Olaf Hering <olaf@xxxxxxxxx>; 
> Tim Deegan <tim@xxxxxxx>;
> Wei Liu <wl@xxxxxxx>
> 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@xxxxxxxxx>
> ---
>  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;
>