Re: [PATCH v2] hwmon: (corsair-psu) Fix linear11 calculation

On Thu, 6 Aug 2026 11:04:50 -0700
Guenter Roeck <[email protected]> wrote:

> On 8/6/26 09:42, Wilken Gottwalt wrote:
> > On Thu, 6 Aug 2026 09:28:34 -0700
> > Guenter Roeck <[email protected]> wrote:
> > 
> >> On 8/6/26 08:56, Wilken Gottwalt wrote:
> >> ...
> >>>>>> +	s64 result = mant * scale;
> >>>>>
> >>>>> Uhm, this is a 32bit multiplicaiton, actully a C gotcha I explain the beginners
> >>>>> in our company. https://godbolt.org/z/eM6TbGG5E
> >>>>>
> >>>>
> >>>> It is, but that is ok and intentional: both mant and exp are guaranteed to be
> >>>> no larger than s16, meaning the result is never larger than s32 and will never
> >>>> overflow.
> >>>>
> >>>>>> -	return (exp >= 0) ? (result << exp) : (result >> -exp);
> >>>>>> +	if (exp >= 0)
> >>>>>> +		result *= (int)(1UL << exp);
> >>>>
> >>>> This is the calculation that can overflow, making it necessary for result to be s64.
> >>>
> >>> Yeah, it was just funny to see in the wild. It made my day. :D
> >>>
> >>
> >> Guess I lost you there. Do you want me to change it ? I could add a comment, or just
> >> type cast mant to s64. Please let me know.
> > 
> > No no, it is fine. I just was surprised to see that famous gotcha. But I guess
> > some AIs may jump on it, ignoring the context. Changing it maybe would prevent
> > noise in the future. It is like that famous mathematical "iff" term, which is
> 
> FWIW, one could argue that it is often misused in the Linux kernel.

I actually don't know. I'm one of the people who considered it a typo. :D

> > used in some places of the kernel. A lot of non-native English speakers report
> > that as a typo. Ahh... just ignore my gibberish.
> > 
> 
> Actually, any reasonable AI should be able to find that this isn't a problem
> given the constraints. Sashiko didn't flag it, and if it did I'd have submitted
> a prompt update. Sashiko is explicitly directed to check for over- and underflows
> in hwmon patches because it happens so often, so I am (reasonably ;-) sure that it
> would flag it if there was a problem.

Yeah, a reasonable AI... So, I guess we will find out in the future.

> If you are ok with the patch, any chance for a Tested-by/Acked-by/Reviewed-by ?

Sorry, I did not test it yet, but I will tomorrow.

Hmm, I really wonder if something will actually change. I never encountered odd
values. Well, except for the really obvious ones. Picking every value one by one
results in every value being from a different sample. I think internally the MCU
samples with about 1000 Hz. But getting that right would be some serious work.

greetings,
Wilken