1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array

Peter Maydell posted 1 patch 6 years, 10 months ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190328152635.2794-1-peter.maydell@linaro.org
Maintainers: Alistair Francis <alistair@alistair23.me>
hw/ssi/xilinx_spips.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Peter Maydell 6 years, 10 months ago 
In the stripe8() function we use a variable length array; however
we know that the maximum length required is MAX_NUM_BUSSES. Use
a fixed-length array and an assert instead.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/ssi/xilinx_spips.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index 16f88f74029..c615058cc1b 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -429,12 +429,14 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d)
 
 static inline void stripe8(uint8_t *x, int num, bool dir)
 {
-    uint8_t r[num];
-    memset(r, 0, sizeof(uint8_t) * num);
+    uint8_t r[MAX_NUM_BUSSES];
     int idx[2] = {0, 0};
     int bit[2] = {0, 7};
     int d = dir;
 
+    assert(num <= MAX_NUM_BUSSES);
+    memset(r, 0, sizeof(uint8_t) * num);
+
     for (idx[0] = 0; idx[0] < num; ++idx[0]) {
         for (bit[0] = 7; bit[0] >= 0; bit[0]--) {
             r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0;
-- 
2.20.1
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Edgar E. Iglesias 6 years, 10 months ago 
+ Francisco

On Thu, 28 Mar. 2019, 16:26 Peter Maydell, <peter.maydell@linaro.org> wrote:

> In the stripe8() function we use a variable length array; however
> we know that the maximum length required is MAX_NUM_BUSSES. Use
> a fixed-length array and an assert instead.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/ssi/xilinx_spips.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 16f88f74029..c615058cc1b 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -429,12 +429,14 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d)
>
>  static inline void stripe8(uint8_t *x, int num, bool dir)
>  {
> -    uint8_t r[num];
> -    memset(r, 0, sizeof(uint8_t) * num);
> +    uint8_t r[MAX_NUM_BUSSES];
>      int idx[2] = {0, 0};
>      int bit[2] = {0, 7};
>      int d = dir;
>
> +    assert(num <= MAX_NUM_BUSSES);
> +    memset(r, 0, sizeof(uint8_t) * num);
> +
>      for (idx[0] = 0; idx[0] < num; ++idx[0]) {
>          for (bit[0] = 7; bit[0] >= 0; bit[0]--) {
>              r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0;
> --
> 2.20.1
>
>
Re: [Qemu-devel] [Qemu-arm] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Philippe Mathieu-Daudé 6 years, 10 months ago 
Le jeu. 28 mars 2019 16:41, Edgar E. Iglesias <edgar.iglesias@gmail.com> a
écrit :

> + Francisco
>
> On Thu, 28 Mar. 2019, 16:26 Peter Maydell, <peter.maydell@linaro.org>
> wrote:
>
>> In the stripe8() function we use a variable length array; however
>> we know that the maximum length required is MAX_NUM_BUSSES. Use
>> a fixed-length array and an assert instead.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/ssi/xilinx_spips.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
>> index 16f88f74029..c615058cc1b 100644
>> --- a/hw/ssi/xilinx_spips.c
>> +++ b/hw/ssi/xilinx_spips.c
>> @@ -429,12 +429,14 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d)
>>
>>  static inline void stripe8(uint8_t *x, int num, bool dir)
>>  {
>> -    uint8_t r[num];
>> -    memset(r, 0, sizeof(uint8_t) * num);
>> +    uint8_t r[MAX_NUM_BUSSES];
>>      int idx[2] = {0, 0};
>>      int bit[2] = {0, 7};
>>      int d = dir;
>>
>> +    assert(num <= MAX_NUM_BUSSES);
>> +    memset(r, 0, sizeof(uint8_t) * num);
>> +
>>      for (idx[0] = 0; idx[0] < num; ++idx[0]) {
>>          for (bit[0] = 7; bit[0] >= 0; bit[0]--) {
>>              r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0;
>> --
>> 2.20.1
>>
>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

>
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Alistair Francis 6 years, 10 months ago 
On Thu, Mar 28, 2019 at 8:39 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> In the stripe8() function we use a variable length array; however
> we know that the maximum length required is MAX_NUM_BUSSES. Use
> a fixed-length array and an assert instead.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Who's tree should this go through?

Alistair

> ---
>  hw/ssi/xilinx_spips.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 16f88f74029..c615058cc1b 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -429,12 +429,14 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d)
>
>  static inline void stripe8(uint8_t *x, int num, bool dir)
>  {
> -    uint8_t r[num];
> -    memset(r, 0, sizeof(uint8_t) * num);
> +    uint8_t r[MAX_NUM_BUSSES];
>      int idx[2] = {0, 0};
>      int bit[2] = {0, 7};
>      int d = dir;
>
> +    assert(num <= MAX_NUM_BUSSES);
> +    memset(r, 0, sizeof(uint8_t) * num);
> +
>      for (idx[0] = 0; idx[0] < num; ++idx[0]) {
>          for (bit[0] = 7; bit[0] >= 0; bit[0]--) {
>              r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0;
> --
> 2.20.1
>
>
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Stefano Garzarella 6 years, 10 months ago 
On Thu, Mar 28, 2019 at 03:26:35PM +0000, Peter Maydell wrote:
> In the stripe8() function we use a variable length array; however
> we know that the maximum length required is MAX_NUM_BUSSES. Use
> a fixed-length array and an assert instead.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/ssi/xilinx_spips.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

I really think is a good idea to remove variable length arrays!

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks,
Stefano
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Peter Maydell 6 years, 10 months ago 
On Fri, 29 Mar 2019 at 13:13, Stefano Garzarella <sgarzare@redhat.com> wrote:
>
> On Thu, Mar 28, 2019 at 03:26:35PM +0000, Peter Maydell wrote:
> > In the stripe8() function we use a variable length array; however
> > we know that the maximum length required is MAX_NUM_BUSSES. Use
> > a fixed-length array and an assert instead.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  hw/ssi/xilinx_spips.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
>
> I really think is a good idea to remove variable length arrays!
>
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Yeah; we have some but not a vast number, so I thought I'd
whittle down a few easy candidates. Feel free to have a
look at some yourself :-)

thanks
-- PMM
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Stefano Garzarella 6 years, 10 months ago 
On Fri, Mar 29, 2019 at 01:50:04PM +0000, Peter Maydell wrote:
> On Fri, 29 Mar 2019 at 13:13, Stefano Garzarella <sgarzare@redhat.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 03:26:35PM +0000, Peter Maydell wrote:
> > > In the stripe8() function we use a variable length array; however
> > > we know that the maximum length required is MAX_NUM_BUSSES. Use
> > > a fixed-length array and an assert instead.
> > >
> > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > > ---
> > >  hw/ssi/xilinx_spips.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> >
> > I really think is a good idea to remove variable length arrays!
> >
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
> 
> Yeah; we have some but not a vast number, so I thought I'd
> whittle down a few easy candidates. Feel free to have a
> look at some yourself :-)

I'll take a look for others candidates :)

Thanks,
Stefano
Re: [Qemu-devel] [PATCH for-4.1] hw/ssi/xilinx_spips: Avoid variable length array
Posted by Francisco Iglesias 6 years, 10 months ago 
On [2019 Mar 28] Thu 15:26:35, Peter Maydell wrote:
> In the stripe8() function we use a variable length array; however
> we know that the maximum length required is MAX_NUM_BUSSES. Use
> a fixed-length array and an assert instead.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

> ---
>  hw/ssi/xilinx_spips.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
> index 16f88f74029..c615058cc1b 100644
> --- a/hw/ssi/xilinx_spips.c
> +++ b/hw/ssi/xilinx_spips.c
> @@ -429,12 +429,14 @@ static void xlnx_zynqmp_qspips_reset(DeviceState *d)
>  
>  static inline void stripe8(uint8_t *x, int num, bool dir)
>  {
> -    uint8_t r[num];
> -    memset(r, 0, sizeof(uint8_t) * num);
> +    uint8_t r[MAX_NUM_BUSSES];
>      int idx[2] = {0, 0};
>      int bit[2] = {0, 7};
>      int d = dir;
>  
> +    assert(num <= MAX_NUM_BUSSES);
> +    memset(r, 0, sizeof(uint8_t) * num);
> +
>      for (idx[0] = 0; idx[0] < num; ++idx[0]) {
>          for (bit[0] = 7; bit[0] >= 0; bit[0]--) {
>              r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0;
> -- 
> 2.20.1
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.