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
+ 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 > >
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> >
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 > >
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
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
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
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 > >
© 2016 - 2024 Red Hat, Inc.