Re: [PATCH v2 2/3] hwmon: (isl28022) Fix integer overflow in power calculation on 32-bit

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

On Tue, 7 Apr 2026 21:21:31 +0000
"Pradhan, Sanman" <sanman.pradhan@hpe.com> wrote:

> From: Sanman Pradhan <psanman@juniper.net>
> 
> Thanks for the review.
> 
> Yes, I checked this.
> 
> In this driver, gain is limited to {1, 2, 4, 8} by
> isl28022_read_properties(), and regval is a 16-bit register value
> (max 65535). The worst-case numerator is:
> 
>   51200000 * 8 * 65535 = 26843136000000
> 
> which is well below U64_MAX, so the multiply cannot overflow.
> 
> I'll switch to min_t(u64, ...) here to make the type handling explicit
> for the u64 result of div_u64(), if that's ok, and send a v3.

No, use min() not min_t().

min_t() doesn't make the type handling 'explicit', it just casts both
values to the specified type and lets you live with any consequences.
min() attempts to stop you doing 'really silly thing' (mostly trying
to stop negative signed values becoming very large signed values).

Even if the compiler generates a signedness warning from min()
replacing it with min_t() really ought to be a last resort.
Not the least of the problems is that people have a habit of using
the type they want for the result, so you'll find:
	x = clamp_t(u8, y, 0, 255);
That is just:
	x = clamp((u8)y, (u8)0, (u8)255);
which just masks the high bits instead of the intended saturation.

	David

> 
> Thank you.
> 
> Regards,
> Sanman Pradhan