| Message ID | 20210318154738.27094-1-borntraeger@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | intc/i8259: avoid (false positive) gcc warning | expand |
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: 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; - } - #ifdef DEBUG_IRQ_LATENCY printf("IRQ%d latency=%0.3fus\n", irq, ? Paolo
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; > - } > - > #ifdef DEBUG_IRQ_LATENCY > printf("IRQ%d latency=%0.3fus\n", > irq, > > > ? > > Paolo >
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 :)
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
diff --git a/hw/intc/i8259.c b/hw/intc/i8259.c index 344fd04db14d..ade6fb726faf 100644 --- a/hw/intc/i8259.c +++ b/hw/intc/i8259.c @@ -176,7 +176,7 @@ 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, irq2 = -1, intno; irq = pic_get_irq(s); if (irq >= 0) {
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. Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> --- hw/intc/i8259.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)