On 2/25/2025 1:09 PM, Christophe JAILLET wrote:
> Le 25/02/2025 à 21:17, Easwar Hariharan a écrit :
>> Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced
>> secs_to_jiffies(). As the value here is a multiple of 1000, use
>> secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication
>>
>> This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with
>> the following Coccinelle rules:
>>
>> @depends on patch@ expression E; @@
>>
>> -msecs_to_jiffies(E * 1000)
>> +secs_to_jiffies(E)
>>
>> @depends on patch@ expression E; @@
>>
>> -msecs_to_jiffies(E * MSEC_PER_SEC)
>> +secs_to_jiffies(E)
>>
>> While here, remove the no-longer necessary check for range since there's
>> no multiplication involved.
>
> I'm not sure this is correct.
> Now you multiply by HZ and things can still overflow.
>
>
> Hoping I got casting right:
>
> #define MSEC_PER_SEC 1000L
> #define HZ 100
>
>
> #define secs_to_jiffies(_secs) (unsigned long)((_secs) * HZ)
>
> static inline unsigned long _msecs_to_jiffies(const unsigned int m)
> {
> return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);
> }
>
> int main() {
>
> int n = INT_MAX - 5;
>
> printf("res = %ld\n", secs_to_jiffies(n));
> printf("res = %ld\n", _msecs_to_jiffies(1000 * n));
>
> return 0;
> }
>
>
> gives :
>
> res = -600
> res = 429496130
>
> with msec, the previous code would catch the overflow, now it overflows silently.
>
> untested, but maybe:
> if (result.uint_32 > INT_MAX / HZ)
> goto out_of_range;
>
> ?
>
> CJ
>
Thanks for the review! I was able to replicate your results, I'll try this range check
and get back.
Thanks,
Easwar (he/him)
