2020-05-07 05:21:43

by Chen Tao

Subject: [PATCH] input/misc/drv260x: Remove a useless comparison

Fix the following warning:

'mode' and 'library' are u32, they are never be negative,
DRV260X_LRA_MODE and DRV260X_LIB_EMPTY are 0x00, the comparison
is always false.

drivers/input/misc/drv260x.c:478:20: warning:
comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (haptics->mode < DRV260X_LRA_MODE ||
drivers/input/misc/drv260x.c:490:23: warning:
comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (haptics->library < DRV260X_LIB_EMPTY ||

Reported-by: Hulk Robot <[email protected]>
Signed-off-by: ChenTao <[email protected]>
---
drivers/input/misc/drv260x.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 79d7fa710a71..0078f742df5c 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -475,8 +475,7 @@ static int drv260x_probe(struct i2c_client *client,
return error;
}

- if (haptics->mode < DRV260X_LRA_MODE ||
- haptics->mode > DRV260X_ERM_MODE) {
+ if (haptics->mode > DRV260X_ERM_MODE) {
dev_err(dev, "Vibrator mode is invalid: %i\n", haptics->mode);
return -EINVAL;
}
@@ -487,8 +486,7 @@ static int drv260x_probe(struct i2c_client *client,
return error;
}

- if (haptics->library < DRV260X_LIB_EMPTY ||
- haptics->library > DRV260X_ERM_LIB_F) {
+ if (haptics->library > DRV260X_ERM_LIB_F) {
dev_err(dev,
"Library value is invalid: %i\n", haptics->library);
return -EINVAL;
--
2.22.0

2020-05-07 19:31:44

by Dmitry Torokhov

Subject: Re: [PATCH] input/misc/drv260x: Remove a useless comparison

Hi,

On Thu, May 07, 2020 at 01:16:17PM +0800, ChenTao wrote:
> Fix the following warning:
>
> 'mode' and 'library' are u32, they are never be negative,
> DRV260X_LRA_MODE and DRV260X_LIB_EMPTY are 0x00, the comparison
> is always false.


The fact that the symbolic names resolve to 0 is just a happenstance. We
should not be writing code based on concrete values in this case. I
wish we could tell the compiler to ignore this kind of cases, where we
want to test a range of values, and do not care about concrete numeric
bounds of said range.

>
> drivers/input/misc/drv260x.c:478:20: warning:
> comparison of unsigned expression < 0 is always false [-Wtype-limits]
> if (haptics->mode < DRV260X_LRA_MODE ||
> drivers/input/misc/drv260x.c:490:23: warning:
> comparison of unsigned expression < 0 is always false [-Wtype-limits]
> if (haptics->library < DRV260X_LIB_EMPTY ||

Are we planning on enabling this warning in kernel by default?

Thanks.

--
Dmitry