Re: [input:next 2732/2735] drivers/input/touchscreen/resistive-adc-touch.c:84 grts_cb() error: uninitialized symbol 'z2'.
flat view
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2021-06-01 06:21:20
Also in:
oe-kbuild-all
Subsystem:
generic resistive touchscreen adc driver, input (keyboard, mouse, joystick, touchscreen) drivers, the rest · Maintainers:
Eugen Hristev, Dmitry Torokhov, Linus Torvalds
On Thu, May 27, 2021 at 01:41:57PM +0300, Dan Carpenter wrote:
On Thu, May 27, 2021 at 12:19:51PM +0200, Oleksij Rempel wrote:quoted
Hi, On Thu, May 27, 2021 at 12:30:00PM +0300, Dan Carpenter wrote:quoted
tree: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next head: 6cf3b3abbf0b3b778138c0f8936aa7820af62cfc commit: fb082cd59afa7fbd3a610e9835050675040b1b99 [2732/2735] Input: resistive-adc-touch - add support for z1 and z2 channels config: x86_64-randconfig-m001-20210526 (attached as .config) compiler: gcc-9 (Debian 9.3.0-22) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <redacted> Reported-by: Dan Carpenter <redacted> New smatch warnings: drivers/input/touchscreen/resistive-adc-touch.c:84 grts_cb() error: uninitialized symbol 'z2'. drivers/input/touchscreen/resistive-adc-touch.c:88 grts_cb() error: uninitialized symbol 'x'. drivers/input/touchscreen/resistive-adc-touch.c:101 grts_cb() error: uninitialized symbol 'y'. Old smatch warnings: drivers/input/touchscreen/resistive-adc-touch.c:101 grts_cb() error: uninitialized symbol 'x'. drivers/input/touchscreen/resistive-adc-touch.c:109 grts_cb() error: uninitialized symbol 'y'. vim +/z2 +84 drivers/input/touchscreen/resistive-adc-touch.c aa132ffb6b0a18 Eugen Hristev 2018-05-22 56 static int grts_cb(const void *data, void *private) aa132ffb6b0a18 Eugen Hristev 2018-05-22 57 { aa132ffb6b0a18 Eugen Hristev 2018-05-22 58 const u16 *touch_info = data; aa132ffb6b0a18 Eugen Hristev 2018-05-22 59 struct grts_state *st = private; fb082cd59afa7f Oleksij Rempel 2021-05-25 60 unsigned int x, y, press = 0, z1 = 0, z2; fb082cd59afa7f Oleksij Rempel 2021-05-25 61 unsigned int Rt, i; aa132ffb6b0a18 Eugen Hristev 2018-05-22 62 fb082cd59afa7f Oleksij Rempel 2021-05-25 63 for (i = 0; i < ARRAY_SIZE(st->ch) && st->ch[i] != GRTS_CH_NONE; i++) { fb082cd59afa7f Oleksij Rempel 2021-05-25 64 switch (st->ch[i]) { fb082cd59afa7f Oleksij Rempel 2021-05-25 65 case GRTS_CH_X: fb082cd59afa7f Oleksij Rempel 2021-05-25 66 x = touch_info[i]; fb082cd59afa7f Oleksij Rempel 2021-05-25 67 break; fb082cd59afa7f Oleksij Rempel 2021-05-25 68 case GRTS_CH_Y: fb082cd59afa7f Oleksij Rempel 2021-05-25 69 y = touch_info[i]; fb082cd59afa7f Oleksij Rempel 2021-05-25 70 break; fb082cd59afa7f Oleksij Rempel 2021-05-25 71 case GRTS_CH_PRESSURE: fb082cd59afa7f Oleksij Rempel 2021-05-25 72 press = touch_info[i]; fb082cd59afa7f Oleksij Rempel 2021-05-25 73 break; fb082cd59afa7f Oleksij Rempel 2021-05-25 74 case GRTS_CH_Z1: fb082cd59afa7f Oleksij Rempel 2021-05-25 75 z1 = touch_info[i]; fb082cd59afa7f Oleksij Rempel 2021-05-25 76 break; fb082cd59afa7f Oleksij Rempel 2021-05-25 77 case GRTS_CH_Z2: fb082cd59afa7f Oleksij Rempel 2021-05-25 78 z2 = touch_info[i]; fb082cd59afa7f Oleksij Rempel 2021-05-25 79 break; fb082cd59afa7f Oleksij Rempel 2021-05-25 80 } fb082cd59afa7f Oleksij Rempel 2021-05-25 81 } fb082cd59afa7f Oleksij Rempel 2021-05-25 82 fb082cd59afa7f Oleksij Rempel 2021-05-25 83 if (z1) { fb082cd59afa7f Oleksij Rempel 2021-05-25 @84 Rt = z2; It's likely this is a false positive, but we can see why the static checker would complain.The presence of this values was validated on probe. Do we really need extra set them to 0?You don't need to do anything. ;) These are a one time email. I just look it over and hit forward. Currently GCC's uninitialized variable checking is disabled in the kernel otherwise there would be more presure to silence the checker warning.
I find "ununitialzied" warnings to be quite useful. How about we change a bit how we map the channels, and this will help with uninitialized warnings as well. Absolutely not tested, so... Thanks. -- Dmitry Input: resistive-adc-touch - rework mapping of channels From: Dmitry Torokhov <dmitry.torokhov@gmail.com> Instead of iterating over channels establish and use channel map to retrieve data. As a side effect this will silence "uninitialized variable" warnings. Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/resistive-adc-touch.c | 114 ++++++++++------------- 1 file changed, 51 insertions(+), 63 deletions(-)
diff --git a/drivers/input/touchscreen/resistive-adc-touch.c b/drivers/input/touchscreen/resistive-adc-touch.c
index ea1884fb49a1..2102916a37ea 100644
--- a/drivers/input/touchscreen/resistive-adc-touch.c
+++ b/drivers/input/touchscreen/resistive-adc-touch.c@@ -25,12 +25,12 @@ #define GRTS_MAX_CHANNELS 4 enum grts_ch_type { - GRTS_CH_NONE = 0, GRTS_CH_X, GRTS_CH_Y, GRTS_CH_PRESSURE, GRTS_CH_Z1, GRTS_CH_Z2, + GRTS_CH_MAX = GRTS_CH_Z2 + 1 }; /**
@@ -42,7 +42,7 @@ enum grts_ch_type { * @iio_cb: iio_callback buffer for the data * @input: the input device structure that we register * @prop: touchscreen properties struct - * @ch: channels that are defined for the touchscreen + * @ch_map: map of channels that are defined for the touchscreen */ struct grts_state { u32 x_plate_ohms;
@@ -52,37 +52,25 @@ struct grts_state { struct iio_cb_buffer *iio_cb; struct input_dev *input; struct touchscreen_properties prop; - u8 ch[GRTS_MAX_CHANNELS]; + u8 ch_map[GRTS_CH_MAX]; }; static int grts_cb(const void *data, void *private) { const u16 *touch_info = data; struct grts_state *st = private; - unsigned int x, y, press = 0, z1 = 0, z2; - unsigned int Rt, i; - - for (i = 0; i < ARRAY_SIZE(st->ch) && st->ch[i] != GRTS_CH_NONE; i++) { - switch (st->ch[i]) { - case GRTS_CH_X: - x = touch_info[i]; - break; - case GRTS_CH_Y: - y = touch_info[i]; - break; - case GRTS_CH_PRESSURE: - press = touch_info[i]; - break; - case GRTS_CH_Z1: - z1 = touch_info[i]; - break; - case GRTS_CH_Z2: - z2 = touch_info[i]; - break; - } - } + unsigned int x, y, press; + + x = touch_info[st->ch_map[GRTS_CH_X]]; + y = touch_info[st->ch_map[GRTS_CH_X]]; + + if (st->ch_map[GRTS_CH_PRESSURE] < GRTS_MAX_CHANNELS) { + press = touch_info[st->ch_map[GRTS_CH_PRESSURE]]; + } else if (st->ch_map[GRTS_CH_Z1] < GRTS_MAX_CHANNELS) { + unsigned int z1 = touch_info[st->ch_map[GRTS_CH_Z1]]; + unsigned int z2 = touch_info[st->ch_map[GRTS_CH_Z2]]; + unsigned int Rt; - if (z1) { Rt = z2; Rt -= z1; Rt *= st->x_plate_ohms;
@@ -142,60 +130,59 @@ static void grts_disable(void *data) iio_channel_release_all_cb(data); } -static int grts_get_properties(struct grts_state *st, struct device *dev) +static int grts_map_channel(struct grts_state *st, struct device *dev, + enum grts_ch_type type, const char *name, + bool optional) { - int idx, error; - - idx = device_property_match_string(dev, "io-channel-names", "x"); - if (idx < 0) - return idx; - - if (idx >= ARRAY_SIZE(st->ch)) + int idx; + + idx = device_property_match_string(dev, "io-channel-names", name); + if (idx < 0) { + if (!optional) + return idx; + idx = GRTS_MAX_CHANNELS; + } else if (idx >= GRTS_MAX_CHANNELS) { return -EOVERFLOW; + } - st->ch[idx] = GRTS_CH_X; + st->ch_map[type] = idx; + return 0; +} - idx = device_property_match_string(dev, "io-channel-names", "y"); - if (idx < 0) - return idx; +static int grts_get_properties(struct grts_state *st, struct device *dev) +{ + int error; - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_X, "x", false); + if (error) + return error; - st->ch[idx] = GRTS_CH_Y; + error = grts_map_channel(st, dev, GRTS_CH_Y, "y", false); + if (error) + return error; /* pressure is optional */ - idx = device_property_match_string(dev, "io-channel-names", "pressure"); - if (idx >= 0) { - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_PRESSURE, "pressure", true); + if (error) + return error; - st->ch[idx] = GRTS_CH_PRESSURE; + if (st->ch_map[GRTS_CH_PRESSURE] < GRTS_MAX_CHANNELS) { st->pressure = true; - return 0; } /* if no pressure is defined, try optional z1 + z2 */ - idx = device_property_match_string(dev, "io-channel-names", "z1"); - if (idx < 0) - return 0; - - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; + error = grts_map_channel(st, dev, GRTS_CH_Z1, "z1", true); + if (error) + return error; - st->ch[idx] = GRTS_CH_Z1; + if (st->ch_map[GRTS_CH_Z1] >= GRTS_MAX_CHANNELS) + return 0; /* if z1 is provided z2 is not optional */ - idx = device_property_match_string(dev, "io-channel-names", "z2"); - if (idx < 0) - return idx; - - if (idx >= ARRAY_SIZE(st->ch)) - return -EOVERFLOW; - - st->ch[idx] = GRTS_CH_Z2; - st->pressure = true; + error = grts_map_channel(st, dev, GRTS_CH_Z2, "z2", true); + if (error) + return error; error = device_property_read_u32(dev, "touchscreen-x-plate-ohms",
@@ -205,6 +192,7 @@ static int grts_get_properties(struct grts_state *st, struct device *dev) return error; } + st->pressure = true; return 0; }