Re: [PATCH] w1: therm: Use clamp_t to simplify int_to_short helper

[David Laight <david.laight.linux@gmail.com>]

On Sun,  9 Nov 2025 13:59:55 +0100
Thorsten Blum <thorsten.blum@linux.dev> wrote:

> Use clamp_t() instead of manually casting the return value.
> 
> Replace sprintf() with sysfs_emit() to improve sysfs show functions
> while we're at it.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  drivers/w1/slaves/w1_therm.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
> index 9ccedb3264fb..cf686e6ba3d5 100644
> --- a/drivers/w1/slaves/w1_therm.c
> +++ b/drivers/w1/slaves/w1_therm.c
> @@ -961,9 +961,8 @@ static inline int temperature_from_RAM(struct w1_slave *sl, u8 rom[9])
>   */
>  static inline s8 int_to_short(int i)
>  {
> -	/* Prepare to cast to short by eliminating out of range values */
> -	i = clamp(i, MIN_TEMP, MAX_TEMP);
> -	return (s8) i;
> +	/* Cast to short by eliminating out of range values */
                   ^^^^^ no shorts here...
> +	return clamp_t(s8, i, MIN_TEMP, MAX_TEMP);

That is just plain broken.
clamp_t() really shouldn't have been allowed to exist.
That is a typical example of how it gets misused.
(min_t() and max_t() get misused the same way.)

Think what happens when i is 256.
The code should just be:

	return clamp(i, MIN_TEMP, MAX_TEMP);

No casts anywhere.
I'm not even sure the return type (s8) makes any sense.
It is quite likely that the code will be better if it is 'int'.
The fact that the domain in inside -128..127 doesn't mean that
the correct type for a variable isn't 'int'.

	David

>  }