Message ID | 20210319142308.3980259-1-philmd@redhat.com |
---|---|
State | New |
Headers | show |
Series | [for,6.0,v4] hw/intc/i8259: Refactor pic_read_irq() to avoid uninitialized variable | expand |
On 3/19/21 3:23 PM, Philippe Mathieu-Daudé wrote: > Some compiler versions are smart enough to detect a potentially > uninitialized variable, but are not smart enough to detect that this > cannot happen due to the code flow: > > ../hw/intc/i8259.c: In function ‘pic_read_irq’: > ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > 203 | irq = irq2 + 8; > | ~~~~^~~~~~~~~~ > > Restrict irq2 variable use to the inner statement. > > Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq") > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > Since v3: > - Declare irq2 really local (Richard) I forgot to keep: Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
On Fri, Mar 19, 2021 at 03:23:08PM +0100, Philippe Mathieu-Daudé wrote: > Some compiler versions are smart enough to detect a potentially > uninitialized variable, but are not smart enough to detect that this > cannot happen due to the code flow: > > ../hw/intc/i8259.c: In function ‘pic_read_irq’: > ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] > 203 | irq = irq2 + 8; > | ~~~~^~~~~~~~~~ > > Restrict irq2 variable use to the inner statement. > > Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq") > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Looks ok. Acked-by: Michael S. Tsirkin <mst@redhat.com> Paolo, you merging this? > --- > Since v3: > - Declare irq2 really local (Richard) > Since v2: > - Remove pic_intack() call (Zoltan) > --- > hw/intc/i8259.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c > index 344fd04db14..d67c4710f15 100644 > --- a/hw/intc/i8259.c > +++ b/hw/intc/i8259.c > @@ -176,12 +176,13 @@ static void pic_intack(PICCommonState *s, int irq) > int pic_read_irq(DeviceState *d) > { > PICCommonState *s = PIC_COMMON(d); > - int irq, irq2, intno; > + int irq, intno; > > irq = pic_get_irq(s); > if (irq >= 0) { > if (irq == 2) { > - irq2 = pic_get_irq(slave_pic); > + int irq2 = pic_get_irq(slave_pic); > + > if (irq2 >= 0) { > pic_intack(slave_pic, irq2); > } else { > @@ -189,20 +190,18 @@ int pic_read_irq(DeviceState *d) > irq2 = 7; > } > intno = slave_pic->irq_base + irq2; > + pic_intack(s, irq); > + irq = irq2 + 8; > } else { > intno = s->irq_base + irq; > + pic_intack(s, irq); > } > - pic_intack(s, irq); > } else { > /* spurious IRQ on host controller */ > irq = 7; > intno = s->irq_base + irq; > } > > - if (irq == 2) { > - irq = irq2 + 8; > - } > - > #ifdef DEBUG_IRQ_LATENCY > printf("IRQ%d latency=%0.3fus\n", > irq, > -- > 2.26.2
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c index 344fd04db14..d67c4710f15 100644 --- a/hw/intc/i8259.c +++ b/hw/intc/i8259.c @@ -176,12 +176,13 @@ static void pic_intack(PICCommonState *s, int irq) int pic_read_irq(DeviceState *d) { PICCommonState *s = PIC_COMMON(d); - int irq, irq2, intno; + int irq, intno; irq = pic_get_irq(s); if (irq >= 0) { if (irq == 2) { - irq2 = pic_get_irq(slave_pic); + int irq2 = pic_get_irq(slave_pic); + if (irq2 >= 0) { pic_intack(slave_pic, irq2); } else { @@ -189,20 +190,18 @@ int pic_read_irq(DeviceState *d) irq2 = 7; } intno = slave_pic->irq_base + irq2; + pic_intack(s, irq); + irq = irq2 + 8; } else { intno = s->irq_base + irq; + pic_intack(s, irq); } - pic_intack(s, irq); } else { /* spurious IRQ on host controller */ irq = 7; intno = s->irq_base + irq; } - if (irq == 2) { - irq = irq2 + 8; - } - #ifdef DEBUG_IRQ_LATENCY printf("IRQ%d latency=%0.3fus\n", irq,
Some compiler versions are smart enough to detect a potentially uninitialized variable, but are not smart enough to detect that this cannot happen due to the code flow: ../hw/intc/i8259.c: In function ‘pic_read_irq’: ../hw/intc/i8259.c:203:13: error: ‘irq2’ may be used uninitialized in this function [-Werror=maybe-uninitialized] 203 | irq = irq2 + 8; | ~~~~^~~~~~~~~~ Restrict irq2 variable use to the inner statement. Fixes: 78ef2b6989f ("i8259: Reorder intack in pic_read_irq") Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- Since v3: - Declare irq2 really local (Richard) Since v2: - Remove pic_intack() call (Zoltan) --- hw/intc/i8259.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)