René Scharfe <l....@web.de> 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.

Reply via email to