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 <borntrae...@de.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Looks ok.
Acked-by: Michael S. Tsirkin <m...@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
