Richard Henderson <richard.hender...@linaro.org> writes: > On 11/20/19 6:30 PM, Fangrui Song wrote: >> On 2019-11-20, Juan Quintela wrote: >>> Markus Armbruster <arm...@redhat.com> wrote: >>>> Fangrui Song <i...@maskray.me> writes: [...] >>>>> diff --git a/util/cutils.c b/util/cutils.c >>>>> index fd591cadf0..2b4484c015 100644 >>>>> --- a/util/cutils.c >>>>> +++ b/util/cutils.c >>>>> @@ -239,10 +239,10 @@ static int do_strtosz(const char *nptr, const char >>>>> **end, >>>>> goto out; >>>>> } >>>>> /* >>>>> - * Values >= 0xfffffffffffffc00 overflow uint64_t after their trip >>>>> + * Values > nextafter(0x1p64, 0) overflow uint64_t after their trip >>>>> * through double (53 bits of precision). >>>>> */ >>>>> - if ((val * mul >= 0xfffffffffffffc00) || val < 0) { >>>>> + if ((val * mul > nextafter(0x1p64, 0)) || val < 0) { >>>>> retval = -ERANGE; >>>>> goto out; >>>>> } >>> >>> This comment was really bad (it says the same that the code). >>> On the other hand, I can *kind of* understand what does 0xffff<more >>> f's here>. >>> >>> But I am at a complete loss about what value is: >>> >>> nextafter(0x1p64, 0). >>> >>> Can we put what value is that instead? >> >> It is a C99 hexadecimal floating-point literal. >> 0x1p64 represents hex fraction 1.0 scaled by 2**64, that is 2**64. >> >> We can write this as `val * mul > 0xfffffffffffff800p0`, but I feel that >> counting the number of f's is error-prone and is not fun. >> >> (We cannot use val * mul >= 0x1p64. >> If FLT_EVAL_METHOD == 2, the intermediate computation val * mul will be >> performed at long double precision, val * mul may not by representable >> by a double and will overflow as (double)0x1p64.) > > I agree about not spelling out the f's, or the 0x800 at the end. That's > something that the compiler can do for us, resolving this standard library > function at compile-time. > > We just need a better comment. Perhaps: > > /* > * Values near UINT64_MAX overflow to 2**64 when converting > * to double precision. Compare against the maximum representable > * double precision value below 2**64, computed as "the next value > * after 2**64 (0x1p64) in the direction of 0". > */
Yes, please.