Re: [bug report] Input: add driver for Hynitron cstxxx touchscreens

[Dan Carpenter <error27@gmail.com>]

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