| Message ID | 1296470546-16488-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | c84a88d8cb298b6757ad01a12a8bbba66cb6eaa2 |
| Headers | show |
Thanks, applied. On Mon, Jan 31, 2011 at 10:42 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > The Ubuntu 10.10 gcc for ARM complains that we might be overrunning > the cpu_irqs[][] array: silence this by correcting the bounds on the > loop. (In fact we would not have overrun the array because bit > MAX_PILS in pil_pending and irl_out will always be 0.) > > Also add a comment about why the loop's lower bound is OK. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > I've tested that with this change we still boot the sparc > Debian image from http://people.debian.org/~aurel32/qemu/sparc/ > and the change makes sense according to my understanding of > http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt > > hw/slavio_intctl.c | 7 ++++++- > 1 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c > index fd69354..a83e5b8 100644 > --- a/hw/slavio_intctl.c > +++ b/hw/slavio_intctl.c > @@ -289,7 +289,12 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s, int set_irqs) > pil_pending |= (s->slaves[i].intreg_pending & CPU_SOFTIRQ_MASK) >> 16; > > if (set_irqs) { > - for (j = MAX_PILS; j > 0; j--) { > + /* Since there is not really an interrupt 0 (and pil_pending > + * and irl_out bit zero are thus always zero) there is no need > + * to do anything with cpu_irqs[i][0] and it is OK not to do > + * the j=0 iteration of this loop. > + */ > + for (j = MAX_PILS-1; j > 0; j--) { > if (pil_pending & (1 << j)) { > if (!(s->slaves[i].irl_out & (1 << j))) { > qemu_irq_raise(s->cpu_irqs[i][j]); > -- > 1.7.1 > >
diff --git a/hw/slavio_intctl.c b/hw/slavio_intctl.c index fd69354..a83e5b8 100644 --- a/hw/slavio_intctl.c +++ b/hw/slavio_intctl.c @@ -289,7 +289,12 @@ static void slavio_check_interrupts(SLAVIO_INTCTLState *s, int set_irqs) pil_pending |= (s->slaves[i].intreg_pending & CPU_SOFTIRQ_MASK) >> 16; if (set_irqs) { - for (j = MAX_PILS; j > 0; j--) { + /* Since there is not really an interrupt 0 (and pil_pending + * and irl_out bit zero are thus always zero) there is no need + * to do anything with cpu_irqs[i][0] and it is OK not to do + * the j=0 iteration of this loop. + */ + for (j = MAX_PILS-1; j > 0; j--) { if (pil_pending & (1 << j)) { if (!(s->slaves[i].irl_out & (1 << j))) { qemu_irq_raise(s->cpu_irqs[i][j]);
The Ubuntu 10.10 gcc for ARM complains that we might be overrunning the cpu_irqs[][] array: silence this by correcting the bounds on the loop. (In fact we would not have overrun the array because bit MAX_PILS in pil_pending and irl_out will always be 0.) Also add a comment about why the loop's lower bound is OK. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- I've tested that with this change we still boot the sparc Debian image from http://people.debian.org/~aurel32/qemu/sparc/ and the change makes sense according to my understanding of http://www.ibiblio.org/pub/historic-linux/early-ports/Sparc/NCR/NCR89C105.txt hw/slavio_intctl.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)