Re: [PATCH 2/2] hwmon: (w83791d) Drop unnecessary compare statements

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Guenter,

On Wed, 19 Sep 2012 11:30:52 -0700, Guenter Roeck wrote:
> The following build warnings are seen with -Wextra.
> 
> w83791d.c: In function ‘store_temp_target’:
> w83791d.c:858:2: warning: comparison of unsigned expression < 0 is always false
> w83791d.c: In function ‘store_temp_tolerance’:
> w83791d.c:920:2: warning: comparison of unsigned expression < 0 is always false
> 
> Drop the unnecessary comparisons.
> 
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
>  drivers/hwmon/w83791d.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/hwmon/w83791d.c b/drivers/hwmon/w83791d.c
> index 9ade4d4..566ac44 100644
> --- a/drivers/hwmon/w83791d.c
> +++ b/drivers/hwmon/w83791d.c
> @@ -254,13 +254,11 @@ static u8 fan_to_reg(long rpm, int div)
>  				 ((val) + 250) / 500 * 128)
>  
>  /* for thermal cruise target temp, 7-bits, LSB = 1 degree Celsius */
> -#define TARGET_TEMP_TO_REG(val)		((val) < 0 ? 0 : \
> -					(val) >= 127000 ? 127 : \
> +#define TARGET_TEMP_TO_REG(val)		((val) >= 127000 ? 127 : \
>  					((val) + 500) / 1000)

The correct fix here is to accept negative input values from the user
and clamp them to 0. There's no way the user can know that the device
won't support negative target temperatures while it supports negative
input temperatures.

>  
>  /* for thermal cruise temp tolerance, 4-bits, LSB = 1 degree Celsius */
> -#define TOL_TEMP_TO_REG(val)		((val) < 0 ? 0 : \
> -					(val) >= 15000 ? 15 : \
> +#define TOL_TEMP_TO_REG(val)		((val) >= 15000 ? 15 : \
>  					((val) + 500) / 1000)


This one is correct.

These macros should be functions, they should use DIV_ROUND_CLOSEST and
SENSOR_LIMIT, but that goes beyond this simple cleanup.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors



[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux