static analyzer showing a couple of potential bugs... about hwsensors

[CozmoNate]

This gpio stuff I think better to keep as is...

[RehabMan]

I wonder whether it is the condition is evaluated as true or false. Statistically, there is a 1/4 chance *ver is less than 0x40, and 3/4 chance that *ver is >= 0x40. Of course, most stack garbage tends to depend on what was there on the stack before, so the value might be fixed and not random (although could change with each build, depending on how the compiler wants to allocate space on the stack).

[CozmoNate]

*ver is set in one of previous calls.

[CozmoNate]

The issue could happen only if using dcb_xpio_table that doesn't set "ver"

So... I really don't know when it happens and what the logic of nouveau developer was in this case..

[RehabMan]

Maybe I'm missing something, but dcb_xpio_table is called after dcb_gpio_parse, so *ver is definitely uninitialized always.

[CozmoNate]

Yes, it's difficult to investigate

static u16 dcb_gpio_entry(struct nouveau_device device, int idx, int ent, u8 *ver, u8 *len)
{
u8 hdr, cnt, xver; / use gpio version for xpio entry parsing */
u16 gpio;

if (!idx--)
    gpio = dcb_gpio_table(device, ver, &hdr, &cnt, len);
else
    gpio = dcb_xpio_table(device, idx, &xver, &hdr, &cnt, len);

if (gpio && ent < cnt)
    return gpio + hdr + (ent * *len);
return 0x0000;

}

we entering dcb_xpio_table only if if (!idx--) == False, but in GeForceSensors idx is always 0, so we get True and entering dcb_gpio_table and setting "ver" to something.

[CozmoNate]

Look for gpio_find in code. Second parameter is idx.

[RehabMan]

Yes, now I see. Thanks. It is "maybe" initialized in dcb_gpio_entry (via call to dcb_gpio_table). Who knows what the intention was here.

[CozmoNate]

Yes. I even don't try to understand all this gpio calls... It may be a full of bugs but seems to work fine in most cases with older GPUs (not Kepler ones)

[RehabMan]

This can be closed if ok with you...

[CozmoNate]

Not a big deal will reopen it if it will be some news.