linux-kernel - Re: [PATCH v2] hwmon: (scpi-hwmon) shows the negative temperature properly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210603105127.GB644104@roeck-us.net>
Date:   Thu, 3 Jun 2021 03:51:27 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Riwen Lu <luriwen@...inos.cn>
Cc:     jdelvare@...e.com, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org, Xin Chen <chenxin@...inos.cn>
Subject: Re: [PATCH v2] hwmon: (scpi-hwmon) shows the negative temperature
 properly

On Thu, Jun 03, 2021 at 04:38:45PM +0800, Riwen Lu wrote:
> The scpi hwmon shows the sub-zero temperature in an unsigned integer,
> which would confuse the users when the machine works in low temperature
> environment. This shows the sub-zero temperature in an signed value and
> users can get it properly from sensors.
> 
> Signed-off-by: Riwen Lu <luriwen@...inos.cn>
> Tested-by: Xin Chen <chenxin@...inos.cn>

Please ask Xin Chen to send a separate Tested-by: tag.

> 
> ---
> Changes since v1:
> - Add judgment for sensor->info.class. If it is TEMPERATURE situation,
>   return the sensor value as a signed value, otherwise return it as a
>   unsigned value.
> ---
>  drivers/hwmon/scpi-hwmon.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/scpi-hwmon.c b/drivers/hwmon/scpi-hwmon.c
> index 25aac40f2764..45a7e319143d 100644
> --- a/drivers/hwmon/scpi-hwmon.c
> +++ b/drivers/hwmon/scpi-hwmon.c
> @@ -99,7 +99,10 @@ scpi_show_sensor(struct device *dev, struct device_attribute *attr, char *buf)
>  
>  	scpi_scale_reading(&value, sensor);
>  
> -	return sprintf(buf, "%llu\n", value);
> +	if (sensor->info.class == TEMPERATURE)
> +		return sprintf(buf, "%lld\n", value);

'value' is u64, so it needs a typecast to s64.

Also, please add a comment ahead of the if statement explaining that temperature
sensor values are treated as signed values based on observation even though that
is not explicitly specified, and because an unsigned u64 temperature does not
really make practical sense.

> +	else

else after return is not needed

Thanks,
Guenter

> +		return sprintf(buf, "%llu\n", value);
>  }
>  
>  static ssize_t
> -- 
> 2.25.1
> 
> 
> No virus found
> 		Checked by Hillstone Network AntiVirus