hwmon: (amc6821) Fix unsigned expression compared with zero

[PATCH] hwmon: (amc6821) Fix unsigned expression compared with zero

Posted by wangkailong@jari.cn 3 years, 5 months ago

Fix the following coccicheck warning:

drivers/hwmon/amc6821.c:215: WARNING: Unsigned expression compared
with zero: reg > 0
drivers/hwmon/amc6821.c:228: WARNING: Unsigned expression compared
with zero: reg > 0

Signed-off-by: KaiLong Wang <wangkailong@jari.cn>
---
 drivers/hwmon/amc6821.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
index 3bfd12ff4b3c..1f5382f8d52b 100644
--- a/drivers/hwmon/amc6821.c
+++ b/drivers/hwmon/amc6821.c
@@ -166,7 +166,7 @@ static struct amc6821_data *amc6821_update_device(struct device *dev)
 	struct amc6821_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
 	int timeout = HZ;
-	u8 reg;
+	int reg;
 	int i;
 
 	mutex_lock(&data->update_lock);
-- 
2.25.1

Re: [PATCH] hwmon: (amc6821) Fix unsigned expression compared with zero

Posted by Randy Dunlap 3 years, 5 months ago


On 11/2/22 19:27, wangkailong@jari.cn wrote:
> Fix the following coccicheck warning:
> 
> drivers/hwmon/amc6821.c:215: WARNING: Unsigned expression compared
> with zero: reg > 0
> drivers/hwmon/amc6821.c:228: WARNING: Unsigned expression compared
> with zero: reg > 0
> 
> Signed-off-by: KaiLong Wang <wangkailong@jari.cn>

Hm. IDGI. What's wrong with comparing an unsigned value to > 0?
I mean, it could be == 0 or > 0.
Please explain.

> ---
>  drivers/hwmon/amc6821.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/amc6821.c b/drivers/hwmon/amc6821.c
> index 3bfd12ff4b3c..1f5382f8d52b 100644
> --- a/drivers/hwmon/amc6821.c
> +++ b/drivers/hwmon/amc6821.c
> @@ -166,7 +166,7 @@ static struct amc6821_data *amc6821_update_device(struct device *dev)
>  	struct amc6821_data *data = dev_get_drvdata(dev);
>  	struct i2c_client *client = data->client;
>  	int timeout = HZ;
> -	u8 reg;
> +	int reg;
>  	int i;
>  
>  	mutex_lock(&data->update_lock);

-- 
~Randy

Re: [PATCH] hwmon: (amc6821) Fix unsigned expression compared with zero

Posted by Guenter Roeck 3 years, 5 months ago

On Wed, Nov 02, 2022 at 07:59:06PM -0700, Randy Dunlap wrote:
> 
> 
> On 11/2/22 19:27, wangkailong@jari.cn wrote:
> > Fix the following coccicheck warning:
> > 
> > drivers/hwmon/amc6821.c:215: WARNING: Unsigned expression compared
> > with zero: reg > 0
> > drivers/hwmon/amc6821.c:228: WARNING: Unsigned expression compared
> > with zero: reg > 0
> > 
> > Signed-off-by: KaiLong Wang <wangkailong@jari.cn>
> 
> Hm. IDGI. What's wrong with comparing an unsigned value to > 0?
> I mean, it could be == 0 or > 0.
> Please explain.

I don't get it either. The real problem with this driver is that error
returns from i2c functions are not checked. However, that problem is not
fixed by this patch. That means the patch would change behavior without
fixing the actual problem.

I wonder what kind of (broken) compiler or analyzer produces above errors.
We'll have to watch out for similar broken "fixes".

Guenter

Re: [PATCH] hwmon: (amc6821) Fix unsigned expression compared with zero

Posted by Randy Dunlap 3 years, 5 months ago


On 11/3/22 07:17, Guenter Roeck wrote:
> On Wed, Nov 02, 2022 at 07:59:06PM -0700, Randy Dunlap wrote:
>>
>>
>> On 11/2/22 19:27, wangkailong@jari.cn wrote:
>>> Fix the following coccicheck warning:
>>>
>>> drivers/hwmon/amc6821.c:215: WARNING: Unsigned expression compared
>>> with zero: reg > 0
>>> drivers/hwmon/amc6821.c:228: WARNING: Unsigned expression compared
>>> with zero: reg > 0
>>>
>>> Signed-off-by: KaiLong Wang <wangkailong@jari.cn>
>>
>> Hm. IDGI. What's wrong with comparing an unsigned value to > 0?
>> I mean, it could be == 0 or > 0.
>> Please explain.
> 
> I don't get it either. The real problem with this driver is that error
> returns from i2c functions are not checked. However, that problem is not
> fixed by this patch. That means the patch would change behavior without
> fixing the actual problem.
> 
> I wonder what kind of (broken) compiler or analyzer produces above errors.
> We'll have to watch out for similar broken "fixes".

It says above that it's a coccicheck warning.

-- 
~Randy

Re: [PATCH] hwmon: (amc6821) Fix unsigned expression compared with zero

Posted by Guenter Roeck 3 years, 5 months ago

On Thu, Nov 03, 2022 at 08:03:35AM -0700, Randy Dunlap wrote:
> 
> 
> On 11/3/22 07:17, Guenter Roeck wrote:
> > On Wed, Nov 02, 2022 at 07:59:06PM -0700, Randy Dunlap wrote:
> >>
> >>
> >> On 11/2/22 19:27, wangkailong@jari.cn wrote:
> >>> Fix the following coccicheck warning:
> >>>
> >>> drivers/hwmon/amc6821.c:215: WARNING: Unsigned expression compared
> >>> with zero: reg > 0
> >>> drivers/hwmon/amc6821.c:228: WARNING: Unsigned expression compared
> >>> with zero: reg > 0
> >>>
> >>> Signed-off-by: KaiLong Wang <wangkailong@jari.cn>
> >>
> >> Hm. IDGI. What's wrong with comparing an unsigned value to > 0?
> >> I mean, it could be == 0 or > 0.
> >> Please explain.
> > 
> > I don't get it either. The real problem with this driver is that error
> > returns from i2c functions are not checked. However, that problem is not
> > fixed by this patch. That means the patch would change behavior without
> > fixing the actual problem.
> > 
> > I wonder what kind of (broken) compiler or analyzer produces above errors.
> > We'll have to watch out for similar broken "fixes".
> 
> It says above that it's a coccicheck warning.
> 

I see, unsigned_lesser_than_zero.cocci. It actually complains that an
unsigned variable is used to hold the return code of a function which
returns an int. In other words, it really tries to warn that the error
return code from that function is not or not properly checked.
The message is misleading for that situation.

Guenter