On Tue, Nov 15, 2022 at 03:36:18PM +0000, Christopher Morgan wrote:
> On Tue, Nov 15, 2022 at 03:43:39PM +0300, Dan Carpenter wrote:
> > [ I sent this a couple weeks back, but it turns out that mutt + msmtp
> > has been silently eating my emails instead of sending them so I'm
> > resending two weeks of email. -dan ]
> >
> > Hello Chris Morgan,
> >
> > The patch 66603243f528: "Input: add driver for Hynitron cstxxx
> > touchscreens" from Oct 28, 2022, leads to the following Smatch static
> > checker warning:
>
> Apologies, but I'm a surprisingly un-skilled programmer.
Heh. Every several years I realize that, "Nope. I still don't
understand pointers."
> I'll be happy to fix the bugs though I just want to make sure I
> understand the problem clearly first.
>
> >
> > drivers/input/touchscreen/hynitron_cstxxx.c:238 cst3xx_bootloader_enter()
> > error: uninitialized symbol 'tmp'.
> >
>
> Does this mean I need to set the inital value of the tmp variable to 0? Looking
> at the code more closely I'm assuming this is the issue because if it runs through
> the loop 5 times and errors each time it will exit the loop without setting a
> value for tmp.
Setting it to zero works. Although I had to look up that
CST3XX_BOOTLDR_CHK_VAL is not zero... But presumably the people who
know the code will understand that right away.
>
> > drivers/input/touchscreen/hynitron_cstxxx.c
> > 209 static int cst3xx_bootloader_enter(struct i2c_client *client)
> > 210 {
> > 211 int err;
> > 212 u8 retry;
> > 213 u32 tmp;
> > 214 unsigned char buf[3];
> > 215
> > 216 for (retry = 0; retry < 5; retry++) {
> > 217 hyn_reset_proc(client, (7 + retry));
> >
> > I would have changed this to a while (retry--) { loop except the retry
> > value probably matters here.
>
> Is that personal prefrence or guidance? The BSP driver did the incremental
> loop 5 times, so that's why I did it here.
>
Yeah. Just a personal preference. I was trying to avoid using the
number 5 twice... Never mind about what I said here. Just initialize
it to zero.
regards,
dan carpenter
