Message ID | 1358870255-32335-1-git-send-email-xi.wang@gmail.com |
---|---|
State | New |
Headers | show |
Il 22/01/2013 16:57, Xi Wang ha scritto: > A correct mask should be `x & (1 << i)', rather than `x && (1 << i)'. > > Also, in C99 signed shift (1 << 31) is undefined behavior, since the > result exceeds INT_MAX; use 1U instead. > > Signed-off-by: Xi Wang <xi.wang@gmail.com> > --- > hw/openrisc_pic.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/openrisc_pic.c b/hw/openrisc_pic.c > index aaeb9a9..4f6d5a0 100644 > --- a/hw/openrisc_pic.c > +++ b/hw/openrisc_pic.c > @@ -26,12 +26,14 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) > { > OpenRISCCPU *cpu = (OpenRISCCPU *)opaque; > int i; > - uint32_t irq_bit = 1 << irq; > + uint32_t irq_bit; > > if (irq > 31 || irq < 0) { > return; > } > > + irq_bit = 1U << irq; > + > if (level) { > cpu->env.picsr |= irq_bit; > } else { > @@ -39,11 +41,11 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) > } > > for (i = 0; i < 32; i++) { > - if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) { > + if ((cpu->env.picsr & (1U << i)) && (cpu->env.picmr & (1U << i))) { > cpu_interrupt(&cpu->env, CPU_INTERRUPT_HARD); > } else { > cpu_reset_interrupt(&cpu->env, CPU_INTERRUPT_HARD); > - cpu->env.picsr &= ~(1 << i); > + cpu->env.picsr &= ~(1U << i); > } > } > } > Ping. Paolo
Hi Xi, On Tue, Jan 22, 2013 at 11:57 PM, Xi Wang <xi.wang@gmail.com> wrote: > A correct mask should be `x & (1 << i)', rather than `x && (1 << i)'. > > Also, in C99 signed shift (1 << 31) is undefined behavior, since the > result exceeds INT_MAX; use 1U instead. > > Signed-off-by: Xi Wang <xi.wang@gmail.com> > --- > hw/openrisc_pic.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/hw/openrisc_pic.c b/hw/openrisc_pic.c > index aaeb9a9..4f6d5a0 100644 > --- a/hw/openrisc_pic.c > +++ b/hw/openrisc_pic.c > @@ -26,12 +26,14 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) > { > OpenRISCCPU *cpu = (OpenRISCCPU *)opaque; > int i; > - uint32_t irq_bit = 1 << irq; > + uint32_t irq_bit; > > if (irq > 31 || irq < 0) { > return; > } > > + irq_bit = 1U << irq; > + Thanks, this part is OK. > if (level) { > cpu->env.picsr |= irq_bit; > } else { > @@ -39,11 +41,11 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) > } > > for (i = 0; i < 32; i++) { > - if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) { > + if ((cpu->env.picsr & (1U << i)) && (cpu->env.picmr & (1U << i))) { > cpu_interrupt(&cpu->env, CPU_INTERRUPT_HARD); > } else { > cpu_reset_interrupt(&cpu->env, CPU_INTERRUPT_HARD); > - cpu->env.picsr &= ~(1 << i); > + cpu->env.picsr &= ~(1U << i); May you please test it? Your change here make qemu-system-or32 can not boot Linux. You can download pre-compiled Linux form here: https://docs.google.com/file/d/0BxeTrz3x0CBLbTNmU0lrV1Y0V0U/edit?usp=sharing or build one yourself. > } > } > } > -- > 1.7.10.4 > At last, openrisc_pic.c is renamed into pic_cpu.c, please update your patch. Regards, Jia.
diff --git a/hw/openrisc_pic.c b/hw/openrisc_pic.c index aaeb9a9..4f6d5a0 100644 --- a/hw/openrisc_pic.c +++ b/hw/openrisc_pic.c @@ -26,12 +26,14 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) { OpenRISCCPU *cpu = (OpenRISCCPU *)opaque; int i; - uint32_t irq_bit = 1 << irq; + uint32_t irq_bit; if (irq > 31 || irq < 0) { return; } + irq_bit = 1U << irq; + if (level) { cpu->env.picsr |= irq_bit; } else { @@ -39,11 +41,11 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level) } for (i = 0; i < 32; i++) { - if ((cpu->env.picsr && (1 << i)) && (cpu->env.picmr && (1 << i))) { + if ((cpu->env.picsr & (1U << i)) && (cpu->env.picmr & (1U << i))) { cpu_interrupt(&cpu->env, CPU_INTERRUPT_HARD); } else { cpu_reset_interrupt(&cpu->env, CPU_INTERRUPT_HARD); - cpu->env.picsr &= ~(1 << i); + cpu->env.picsr &= ~(1U << i); } } }
A correct mask should be `x & (1 << i)', rather than `x && (1 << i)'. Also, in C99 signed shift (1 << 31) is undefined behavior, since the result exceeds INT_MAX; use 1U instead. Signed-off-by: Xi Wang <xi.wang@gmail.com> --- hw/openrisc_pic.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)