[PATCH v2 00/11] misc: Remove variable-length arrays on the stack

[PATCH v2 00/11] misc: Remove variable-length arrays on the stack

[Peter Maydell]

This is a resend of a subset of patches from a series that
Philippe sent out last year:
https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/

Basically I just pulled out the patches which:
 (1) trivially applied on a rebase
 (2) had got a Reviewed-by: or at least an Acked-by:

since these should be good to just apply immediately
(well, as soon as we reopen for 7.2 development).

Given they're a mixed bag, I propose to take these via
the target-arm.next tree, unless anybody specifically
wishes to grab specific patches via some other route.

I might come back and have another look at the other
left-behind patches later, but this gets rid of more
than half of the complaints that a -Wvla build reports.

thanks
-- PMM

Philippe Mathieu-Daudé (11):
  chardev/baum: Replace magic values by X_MAX / Y_MAX definitions
  chardev/baum: Use definitions to avoid dynamic stack allocation
  chardev/baum: Avoid dynamic stack allocation
  io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1
  hw/net/e1000e_core: Use definition to avoid dynamic stack allocation
  hw/ppc/pnv: Avoid dynamic stack allocation
  hw/intc/xics: Avoid dynamic stack allocation
  hw/i386/multiboot: Avoid dynamic stack allocation
  hw/usb/hcd-ohci: Use definition to avoid dynamic stack allocation
  ui/curses: Avoid dynamic stack allocation
  tests/unit/test-vmstate: Avoid dynamic stack allocation

 chardev/baum.c             | 22 +++++++++++++---------
 hw/i386/multiboot.c        |  5 ++---
 hw/intc/xics.c             |  2 +-
 hw/net/e1000e_core.c       |  7 ++++---
 hw/ppc/pnv.c               |  4 ++--
 hw/ppc/spapr.c             |  8 ++++----
 hw/ppc/spapr_pci_nvlink2.c |  2 +-
 hw/usb/hcd-ohci.c          |  7 ++++---
 io/channel-websock.c       |  2 +-
 tests/unit/test-vmstate.c  |  7 +++----
 ui/curses.c                |  2 +-
 11 files changed, 36 insertions(+), 32 deletions(-)

-- 
2.25.1

[PATCH v2 01/11] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 chardev/baum.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/chardev/baum.c b/chardev/baum.c
index 79d618e3504..6d538808a0f 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -87,6 +87,9 @@
 
 #define BUF_SIZE 256
 
+#define X_MAX   84
+#define Y_MAX   1
+
 struct BaumChardev {
     Chardev parent;
 
@@ -244,11 +247,11 @@ static int baum_deferred_init(BaumChardev *baum)
         brlapi_perror("baum: brlapi__getDisplaySize");
         return 0;
     }
-    if (baum->y > 1) {
-        baum->y = 1;
+    if (baum->y > Y_MAX) {
+        baum->y = Y_MAX;
     }
-    if (baum->x > 84) {
-        baum->x = 84;
+    if (baum->x > X_MAX) {
+        baum->x = X_MAX;
     }
 
     con = qemu_console_lookup_by_index(0);
-- 
2.25.1

Re: [PATCH v2 01/11] chardev/baum: Replace magic values by X_MAX / Y_MAX definitions

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> Replace '84' magic value by the X_MAX definition, and '1' by Y_MAX.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> Reviewed-by: Samuel Thibault<samuel.thibault@ens-lyon.org>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   chardev/baum.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 02/11] chardev/baum: Use definitions to avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not
a big value, it is actually 84). Instead of having the compiler
use variable-length array, declare an array able to hold the
maximum 'x * y'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 chardev/baum.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/chardev/baum.c b/chardev/baum.c
index 6d538808a0f..6a210ffd815 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -383,9 +383,9 @@ static int baum_eat_packet(BaumChardev *baum, const uint8_t *buf, int len)
     switch (req) {
     case BAUM_REQ_DisplayData:
     {
-        uint8_t cells[baum->x * baum->y], c;
-        uint8_t text[baum->x * baum->y];
-        uint8_t zero[baum->x * baum->y];
+        uint8_t cells[X_MAX * Y_MAX], c;
+        uint8_t text[X_MAX * Y_MAX];
+        uint8_t zero[X_MAX * Y_MAX];
         int cursor = BRLAPI_CURSOR_OFF;
         int i;
 
@@ -408,7 +408,7 @@ static int baum_eat_packet(BaumChardev *baum, const uint8_t *buf, int len)
         }
         timer_del(baum->cellCount_timer);
 
-        memset(zero, 0, sizeof(zero));
+        memset(zero, 0, baum->x * baum->y);
 
         brlapi_writeArguments_t wa = {
             .displayNumber = BRLAPI_DISPLAY_DEFAULT,
-- 
2.25.1

Re: [PATCH v2 02/11] chardev/baum: Use definitions to avoid dynamic stack allocation

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> We know 'x * y' will be at most 'X_MAX * Y_MAX' (which is not
> a big value, it is actually 84). Instead of having the compiler
> use variable-length array, declare an array able to hold the
> maximum 'x * y'.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> Reviewed-by: Samuel Thibault<samuel.thibault@ens-lyon.org>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   chardev/baum.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 03/11] chardev/baum: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length
array on the stack.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 chardev/baum.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/baum.c b/chardev/baum.c
index 6a210ffd815..0a0d12661a4 100644
--- a/chardev/baum.c
+++ b/chardev/baum.c
@@ -299,7 +299,8 @@ static void baum_chr_accept_input(struct Chardev *chr)
 static void baum_write_packet(BaumChardev *baum, const uint8_t *buf, int len)
 {
     Chardev *chr = CHARDEV(baum);
-    uint8_t io_buf[1 + 2 * len], *cur = io_buf;
+    g_autofree uint8_t *io_buf = g_malloc(1 + 2 * len);
+    uint8_t *cur = io_buf;
     int room;
     *cur++ = ESC;
     while (len--)
-- 
2.25.1

Re: [PATCH v2 03/11] chardev/baum: Avoid dynamic stack allocation

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Reviewed-by: Marc-André Lureau<marcandre.lureau@redhat.com>
> Reviewed-by: Samuel Thibault<samuel.thibault@ens-lyon.org>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   chardev/baum.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 04/11] io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The combined_key[... QIO_CHANNEL_WEBSOCK_GUID_LEN ...] array in
qio_channel_websock_handshake_send_res_ok() expands to a call
to strlen(QIO_CHANNEL_WEBSOCK_GUID), and the compiler doesn't
realize the string is const, so consider combined_key[] being
a variable-length array.

To remove the variable-length array, we provide it a hint to
the compiler by using sizeof() - 1 instead of strlen().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 io/channel-websock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index 9619906ac36..fb4932ade70 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -32,7 +32,7 @@
 
 #define QIO_CHANNEL_WEBSOCK_CLIENT_KEY_LEN 24
 #define QIO_CHANNEL_WEBSOCK_GUID "258EAFA5-E914-47DA-95CA-C5AB0DC85B11"
-#define QIO_CHANNEL_WEBSOCK_GUID_LEN strlen(QIO_CHANNEL_WEBSOCK_GUID)
+#define QIO_CHANNEL_WEBSOCK_GUID_LEN (sizeof(QIO_CHANNEL_WEBSOCK_GUID) - 1)
 
 #define QIO_CHANNEL_WEBSOCK_HEADER_PROTOCOL "sec-websocket-protocol"
 #define QIO_CHANNEL_WEBSOCK_HEADER_VERSION "sec-websocket-version"
-- 
2.25.1

Re: [PATCH v2 04/11] io/channel-websock: Replace strlen(const_str) by sizeof(const_str) - 1

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> The combined_key[... QIO_CHANNEL_WEBSOCK_GUID_LEN ...] array in
> qio_channel_websock_handshake_send_res_ok() expands to a call
> to strlen(QIO_CHANNEL_WEBSOCK_GUID), and the compiler doesn't
> realize the string is const, so consider combined_key[] being
> a variable-length array.
> 
> To remove the variable-length array, we provide it a hint to
> the compiler by using sizeof() - 1 instead of strlen().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Reviewed-by: Daniel P. Berrangé<berrange@redhat.com>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   io/channel-websock.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 05/11] hw/net/e1000e_core: Use definition to avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The compiler isn't clever enough to figure 'min_buf_size'
is a constant, so help it by using a definitions instead.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/net/e1000e_core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 208e3e0d798..82aa61fedcd 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1622,15 +1622,16 @@ e1000e_rx_fix_l4_csum(E1000ECore *core, struct NetRxPkt *pkt)
     }
 }
 
+/* Min. octets in an ethernet frame sans FCS */
+#define MIN_BUF_SIZE 60
+
 ssize_t
 e1000e_receive_iov(E1000ECore *core, const struct iovec *iov, int iovcnt)
 {
     static const int maximum_ethernet_hdr_len = (14 + 4);
-    /* Min. octets in an ethernet frame sans FCS */
-    static const int min_buf_size = 60;
 
     uint32_t n = 0;
-    uint8_t min_buf[min_buf_size];
+    uint8_t min_buf[MIN_BUF_SIZE];
     struct iovec min_iov;
     uint8_t *filter_buf;
     size_t size, orig_size;
-- 
2.25.1

[PATCH v2 06/11] hw/ppc/pnv: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length
array on the stack.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ppc/pnv.c               | 4 ++--
 hw/ppc/spapr.c             | 8 ++++----
 hw/ppc/spapr_pci_nvlink2.c | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d3f77c83672..dd4101e5b65 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -137,7 +137,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
     int smt_threads = CPU_CORE(pc)->nr_threads;
     CPUPPCState *env = &cpu->env;
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
-    uint32_t servers_prop[smt_threads];
+    g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads);
     int i;
     uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
                        0xffffffff, 0xffffffff};
@@ -240,7 +240,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
         servers_prop[i] = cpu_to_be32(pc->pir + i);
     }
     _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
-                       servers_prop, sizeof(servers_prop))));
+                       servers_prop, sizeof(*servers_prop) * smt_threads)));
 }
 
 static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bc9ba6e6dcf..28626efd479 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -177,8 +177,8 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
                                   int smt_threads)
 {
     int i, ret = 0;
-    uint32_t servers_prop[smt_threads];
-    uint32_t gservers_prop[smt_threads * 2];
+    g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads);
+    g_autofree uint32_t *gservers_prop = g_new(uint32_t, smt_threads * 2);
     int index = spapr_get_vcpu_id(cpu);
 
     if (cpu->compat_pvr) {
@@ -196,12 +196,12 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
         gservers_prop[i*2 + 1] = 0;
     }
     ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
-                      servers_prop, sizeof(servers_prop));
+                      servers_prop, sizeof(*servers_prop) * smt_threads);
     if (ret < 0) {
         return ret;
     }
     ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-gserver#s",
-                      gservers_prop, sizeof(gservers_prop));
+                      gservers_prop, sizeof(*gservers_prop) * smt_threads * 2);
 
     return ret;
 }
diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
index 63b476c8f72..2a8a11be1d6 100644
--- a/hw/ppc/spapr_pci_nvlink2.c
+++ b/hw/ppc/spapr_pci_nvlink2.c
@@ -397,7 +397,7 @@ void spapr_phb_nvgpu_populate_pcidev_dt(PCIDevice *dev, void *fdt, int offset,
             continue;
         }
         if (dev == nvslot->gpdev) {
-            uint32_t npus[nvslot->linknum];
+            g_autofree uint32_t *npus = g_new(uint32_t, nvslot->linknum);
 
             for (j = 0; j < nvslot->linknum; ++j) {
                 PCIDevice *npdev = nvslot->links[j].npdev;
-- 
2.25.1

Re: [PATCH v2 06/11] hw/ppc/pnv: Avoid dynamic stack allocation

[Daniel Henrique Barboza]

On 8/19/22 12:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>


Feel free to take it via target-arm.next.



Thanks,


Daniel

> ---
>   hw/ppc/pnv.c               | 4 ++--
>   hw/ppc/spapr.c             | 8 ++++----
>   hw/ppc/spapr_pci_nvlink2.c | 2 +-
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index d3f77c83672..dd4101e5b65 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -137,7 +137,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>       int smt_threads = CPU_CORE(pc)->nr_threads;
>       CPUPPCState *env = &cpu->env;
>       PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
> -    uint32_t servers_prop[smt_threads];
> +    g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads);
>       int i;
>       uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
>                          0xffffffff, 0xffffffff};
> @@ -240,7 +240,7 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>           servers_prop[i] = cpu_to_be32(pc->pir + i);
>       }
>       _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
> -                       servers_prop, sizeof(servers_prop))));
> +                       servers_prop, sizeof(*servers_prop) * smt_threads)));
>   }
>   
>   static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bc9ba6e6dcf..28626efd479 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -177,8 +177,8 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>                                     int smt_threads)
>   {
>       int i, ret = 0;
> -    uint32_t servers_prop[smt_threads];
> -    uint32_t gservers_prop[smt_threads * 2];
> +    g_autofree uint32_t *servers_prop = g_new(uint32_t, smt_threads);
> +    g_autofree uint32_t *gservers_prop = g_new(uint32_t, smt_threads * 2);
>       int index = spapr_get_vcpu_id(cpu);
>   
>       if (cpu->compat_pvr) {
> @@ -196,12 +196,12 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
>           gservers_prop[i*2 + 1] = 0;
>       }
>       ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
> -                      servers_prop, sizeof(servers_prop));
> +                      servers_prop, sizeof(*servers_prop) * smt_threads);
>       if (ret < 0) {
>           return ret;
>       }
>       ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-gserver#s",
> -                      gservers_prop, sizeof(gservers_prop));
> +                      gservers_prop, sizeof(*gservers_prop) * smt_threads * 2);
>   
>       return ret;
>   }
> diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
> index 63b476c8f72..2a8a11be1d6 100644
> --- a/hw/ppc/spapr_pci_nvlink2.c
> +++ b/hw/ppc/spapr_pci_nvlink2.c
> @@ -397,7 +397,7 @@ void spapr_phb_nvgpu_populate_pcidev_dt(PCIDevice *dev, void *fdt, int offset,
>               continue;
>           }
>           if (dev == nvslot->gpdev) {
> -            uint32_t npus[nvslot->linknum];
> +            g_autofree uint32_t *npus = g_new(uint32_t, nvslot->linknum);
>   
>               for (j = 0; j < nvslot->linknum; ++j) {
>                   PCIDevice *npdev = nvslot->links[j].npdev;

Re: [PATCH v2 06/11] hw/ppc/pnv: Avoid dynamic stack allocation

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Acked-by: David Gibson<david@gibson.dropbear.id.au>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/ppc/pnv.c               | 4 ++--
>   hw/ppc/spapr.c             | 8 ++++----
>   hw/ppc/spapr_pci_nvlink2.c | 2 +-
>   3 files changed, 7 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 07/11] hw/intc/xics: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length
array on the stack.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/intc/xics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 5b0b4d96242..dcd021af668 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -567,8 +567,8 @@ static void ics_reset_irq(ICSIRQState *irq)
 static void ics_reset(DeviceState *dev)
 {
     ICSState *ics = ICS(dev);
+    g_autofree uint8_t *flags = g_malloc(ics->nr_irqs);
     int i;
-    uint8_t flags[ics->nr_irqs];
 
     for (i = 0; i < ics->nr_irqs; i++) {
         flags[i] = ics->irqs[i].flags;
-- 
2.25.1

Re: [PATCH v2 07/11] hw/intc/xics: Avoid dynamic stack allocation

[Richard Henderson]

On 8/20/22 01:39, Peter Maydell wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> Use autofree heap allocation instead of variable-length
> array on the stack.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Acked-by: David Gibson<david@gibson.dropbear.id.au>
> Reviewed-by: Greg Kurz<groug@kaod.org>
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   hw/intc/xics.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 08/11] hw/i386/multiboot: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length array on
the stack. Replace the snprintf() call by g_strdup_printf().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/i386/multiboot.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i386/multiboot.c b/hw/i386/multiboot.c
index 0a10089f14b..963e29362e4 100644
--- a/hw/i386/multiboot.c
+++ b/hw/i386/multiboot.c
@@ -163,6 +163,7 @@ int load_multiboot(X86MachineState *x86ms,
     uint8_t *mb_bootinfo_data;
     uint32_t cmdline_len;
     GList *mods = NULL;
+    g_autofree char *kcmdline = NULL;
 
     /* Ok, let's see if it is a multiboot image.
        The header is 12x32bit long, so the latest entry may be 8192 - 48. */
@@ -362,9 +363,7 @@ int load_multiboot(X86MachineState *x86ms,
     }
 
     /* Commandline support */
-    char kcmdline[strlen(kernel_filename) + strlen(kernel_cmdline) + 2];
-    snprintf(kcmdline, sizeof(kcmdline), "%s %s",
-             kernel_filename, kernel_cmdline);
+    kcmdline = g_strdup_printf("%s %s", kernel_filename, kernel_cmdline);
     stl_p(bootinfo + MBI_CMDLINE, mb_add_cmdline(&mbs, kcmdline));
 
     stl_p(bootinfo + MBI_BOOTLOADER, mb_add_bootloader(&mbs, bootloader_name));
-- 
2.25.1

[PATCH v2 09/11] hw/usb/hcd-ohci: Use definition to avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

The compiler isn't clever enough to figure 'width' is a constant,
so help it by using a definitions instead.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/usb/hcd-ohci.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index 895b29fb865..5585fd32ccf 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -805,13 +805,14 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed)
     return 1;
 }
 
+#define HEX_CHAR_PER_LINE 16
+
 static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
 {
     bool print16;
     bool printall;
-    const int width = 16;
     int i;
-    char tmp[3 * width + 1];
+    char tmp[3 * HEX_CHAR_PER_LINE + 1];
     char *p = tmp;
 
     print16 = !!trace_event_get_state_backends(TRACE_USB_OHCI_TD_PKT_SHORT);
@@ -822,7 +823,7 @@ static void ohci_td_pkt(const char *msg, const uint8_t *buf, size_t len)
     }
 
     for (i = 0; ; i++) {
-        if (i && (!(i % width) || (i == len))) {
+        if (i && (!(i % HEX_CHAR_PER_LINE) || (i == len))) {
             if (!printall) {
                 trace_usb_ohci_td_pkt_short(msg, tmp);
                 break;
-- 
2.25.1

[PATCH v2 10/11] ui/curses: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length
array on the stack.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 ui/curses.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/curses.c b/ui/curses.c
index 861d63244c7..de962faa7cd 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -69,7 +69,7 @@ static void curses_update(DisplayChangeListener *dcl,
                           int x, int y, int w, int h)
 {
     console_ch_t *line;
-    cchar_t curses_line[width];
+    g_autofree cchar_t *curses_line = g_new(cchar_t, width);
     wchar_t wch[CCHARW_MAX];
     attr_t attrs;
     short colors;
-- 
2.25.1

[PATCH v2 11/11] tests/unit/test-vmstate: Avoid dynamic stack allocation

[Peter Maydell]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Use autofree heap allocation instead of variable-length
array on the stack.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 tests/unit/test-vmstate.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-vmstate.c b/tests/unit/test-vmstate.c
index 72077b57800..541bb4f63e3 100644
--- a/tests/unit/test-vmstate.c
+++ b/tests/unit/test-vmstate.c
@@ -87,17 +87,16 @@ static void save_buffer(const uint8_t *buf, size_t buf_size)
 static void compare_vmstate(const uint8_t *wire, size_t size)
 {
     QEMUFile *f = open_test_file(false);
-    uint8_t result[size];
+    g_autofree uint8_t *result = g_malloc(size);
 
     /* read back as binary */
 
-    g_assert_cmpint(qemu_get_buffer(f, result, sizeof(result)), ==,
-                    sizeof(result));
+    g_assert_cmpint(qemu_get_buffer(f, result, size), ==, size);
     g_assert(!qemu_file_get_error(f));
 
     /* Compare that what is on the file is the same that what we
        expected to be there */
-    SUCCESS(memcmp(result, wire, sizeof(result)));
+    SUCCESS(memcmp(result, wire, size));
 
     /* Must reach EOF */
     qemu_get_byte(f);
-- 
2.25.1

Re: [PATCH v2 00/11] misc: Remove variable-length arrays on the stack

[Philippe Mathieu-Daudé via]

On 19/8/22 17:39, Peter Maydell wrote:
> This is a resend of a subset of patches from a series that
> Philippe sent out last year:
> https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/
> 
> Basically I just pulled out the patches which:
>   (1) trivially applied on a rebase
>   (2) had got a Reviewed-by: or at least an Acked-by:
> 
> since these should be good to just apply immediately
> (well, as soon as we reopen for 7.2 development).
> 
> Given they're a mixed bag, I propose to take these via
> the target-arm.next tree, unless anybody specifically
> wishes to grab specific patches via some other route.
> 
> I might come back and have another look at the other
> left-behind patches later, but this gets rid of more
> than half of the complaints that a -Wvla build reports.
> 
> thanks
> -- PMM

Thank you Peter!