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

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:

quoted

[ 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.

quoted

       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.

quoted

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