qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Fix incorrect int->float conversions caught by clang -Wimpli


From: Fangrui Song
Subject: Re: [PATCH] Fix incorrect int->float conversions caught by clang -Wimplicit-int-float-conversion
Date: Thu, 21 Nov 2019 09:11:01 -0800

On 2019-11-21, Markus Armbruster wrote:
Richard Henderson <address@hidden> writes:

On 11/20/19 6:30 PM, Fangrui Song wrote:
On 2019-11-20, Juan Quintela wrote:
Markus Armbruster <address@hidden> wrote:
Fangrui Song <address@hidden> 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.

Thanks for the suggestion. Attached a new patch.

Attachment: qemu.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]