René Scharfe <[email protected]> writes:
>>>> To fix it, let's just "unroll" the function (i.e. negate the value if it
>>>> is negative).
>>>
>>> There's also imaxabs(3).
That may be true, but seeing that some platforms wants to see
intmax_t defined in the compat/ layer, I suspect we cannot avoid
having a copy of unrolled implementation somewhere in our code.
>>>> + uval = val < 0 ? -val : val;
>>>> uval *= factor;
>>>> - if (uval > max || labs(val) > uval) {
>>>> + if (uval > max || (val < 0 ? -val : val) > uval) {
>>>> errno = ERANGE;
>>>> return 0;
>>>> }
>>>
>>> So this check uses unsigned arithmetic to find out if the multiplication
>>> overflows, right?...
>> No, this example is wrong...
> ...
> Anyway, the code would be easier to read and ready for any units if it
> used unsigned_mult_overflows; would have saved me time spent painfully
> wading through the math at least.
A patch to use unsigned_mult_overflows() here, on top of the
"unrolled imaxabs" patch we reviewed, would be good to tie a loose
end.
Thanks.
