On Fri, Apr 17, 2020 at 6:45 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > > On Fri, Apr 17, 2020 at 06:15:05PM -0400, Paul Thomas wrote: > > On Fri, Apr 17, 2020 at 5:58 PM coverity-bot <keescook@xxxxxxxxxxxx> wrote: > > > > > > Hello! > > > > > > This is an experimental semi-automated report about issues detected by > > > Coverity from a scan of next-20200417 as part of the linux-next scan project: > > > https://scan.coverity.com/projects/linux-next-weekly-scan > > > > > > You're getting this email because you were associated with the identified > > > lines of code (noted below) that were touched by commits: > > > > > > Tue Apr 14 11:28:42 2020 -0400 > > > 96d7c7b3e654 ("gpio: gpio-pca953x, Add get_multiple function") > > > > > > Coverity reported the following: > > > > > > *** CID 1492652: Uninitialized variables (UNINIT) > > > /drivers/gpio/gpio-pca953x.c: 499 in pca953x_gpio_get_multiple() > > > 493 if (ret < 0) > > > 494 return ret; > > > 495 } > > > 496 /* reg_val is relative to the last read byte, > > > 497 * so only shift the relative bits > > > 498 */ > > > vvv CID 1492652: Uninitialized variables (UNINIT) > > > vvv Using uninitialized value "reg_val". > > > 499 value = (reg_val >> (i % 8)) & 0x01; > > > 500 __assign_bit(i, bits, value); > > > 501 } > > > 502 return ret; > > > 503 } > > > 504 > > Well for this case it is forced on the first pass with > > offset = gc->ngpio; > > so 'i' in the for_each_set_bit() loop will always be at lest 1 less > > than gc->ngpio. > > > > However, I could see how this is a little are hard for a detection > > tool to follow through: > > offset = gc->ngpio; > > for_each_set_bit(i, mask, gc->ngpio) { > > if ((offset >> BANK_SFT) != (i >> BANK_SFT)) { > > Ah yeah, it can't see through the bounds of the "if" and offset and > the shifts. > > > These tools are very cool, and I'd like fix the detection one way or > > another. Any suggestions on a better syntax? > > Well... I don't think it's going to improve its checking of that loop. I > can just mark it false-positive and ignore it. :) (Or you can init > reg_val to zero at the top. *shrug*) Yeah, init to zero sounds good to me, one instruction is nothing compared to what this saves by not doing the same i2c transaction multiple times. Bart do you want the original patch updated or something else? Nice work on Coverity Kees! thanks, Paul > > Thanks for looking at it! > > -Kees > > -- > Kees Cook
