Re: [PATCH] intc/i8259: avoid (false positive) gcc warning

[BALATON Zoltan]

On Thu, 18 Mar 2021, Philippe Mathieu-Daudé wrote:
> On 3/18/21 5:11 PM, Christian Borntraeger wrote:
> > On 18.03.21 17:03, Paolo Bonzini wrote:
> > > On 18/03/21 16:47, Christian Borntraeger wrote:
> > > > some copiler 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;
> > > >         |         ~~~~^~~~~~~~~~
> > > >
> > > > Let us initialize irq2 to -1 to avoid this warning as the most simple
> > > > solution.
> > >
> > > What about:
> >
> > This also works, but if you want to go down that path then it would be
> > good if you
> > could do this patch as I do not have the testing infrastructure to do
> > proper x86
> > changes.
> > > diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c
> > > index 344fd04db1..bf28c179de 100644
> > > --- a/hw/intc/i8259.c
> > > +++ b/hw/intc/i8259.c
> > > @@ -189,20 +189,18 @@ int pic_read_irq(DeviceState *d)
> > >                   irq2 = 7;
> > >               }
> > >               intno = slave_pic->irq_base + irq2;
> > > +            irq = irq2 + 8;
> > > +            pic_intack(s, 2);
> > >           } 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;
> > > -    }
> > > -
>
> This looks like the patch I just sent :)

Except this is simpler and more straightforward. I like this better FWIW.

Regards,
BALATON Zoltan