[PATCH 06/10] qobject: Fix qnum_to_string() to use sufficient precision

[Markus Armbruster]

We should serialize numbers to JSON so that they deserialize back to
the same number.  We fail to do so.

The culprit is qnum_to_string(): it uses format %f with trailing '0'
trimmed.  Results in pretty output for "nice" numbers, but is prone to
nasty rounding errors.  For instance, numbers between 0 and 0.0000005
get flushed to zero.

Where exactly the incorrect rounding can bite is tiresome to gauge.
Here's my take.

* In QMP output, type 'number':

  - query-blockstats value avg_rd_queue_depth

  - QMP query-migrate values mbps, cache-miss-rate, encoding-rate,
    busy-rate, compression-rate.

  Relatively harmless, I guess.

* In tracing QMP input.  Harmless.

* In qemu-ga output, type 'number': guest-get-users value login-time.
  Harmless.

* In output of HMP qom-get.  Harmless.

Not affected, because double values don't actually occur there (I
think):

* QMP output, type 'any':

  * qom-get value

  * qom-list, qom-list-properties value default-value

  * query-cpu-model-comparison, query-cpu-model-baseline,
    query-cpu-model-expansion value props.

* qemu-img --output json output.

* "json:" pseudo-filenames generated by bdrv_refresh_filename().

* The rbd block driver's "=keyvalue-pairs" hack.

* In -object help on property default values.  Aside: use of JSON
  feels inappropriate here.

* Output of HMP qom-get.

* Argument conversion to QemuOpts for qdev_device_add() and HMP with
  qemu_opts_from_qdict()

  QMP and HMP device_add, virtio-net failover primary creation,
  xen-usb "usb-host" creation, HMP netdev_add, object_add.

* The uses of qobject_input_visitor_new_flat_confused()

  As far as I can tell, none of the visited types contain double
  values.

* Dumping ImageInfoSpecific with dump_qobject()

Fix by formatting with %.17g.  17 decimal digits always suffice for
IEEE double.

The change to expected test output illustrates the effect: the
rounding errors are gone, but some seemingly "nice" numbers now get
converted to not so nice strings, e.g. 0.42 to "0.41999999999999998".
This is because 0.42 is not representable exactly in double.  It's
more accurate in this example than strictly necessary, though.

If ugly accuracy bothers us, we can we can try using the least number
of digits that still converts back to the same double.  In this
example, "0.42" would do.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qnum.c      | 24 +++---------------------
 tests/check-qjson.c |  8 ++++----
 tests/check-qnum.c  |  4 ++--
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/qobject/qnum.c b/qobject/qnum.c
index 7012fc57f2..bf1240ecec 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -161,37 +161,19 @@ double qnum_get_double(QNum *qn)
 
 char *qnum_to_string(QNum *qn)
 {
-    char *buffer;
-    int len;
-
     switch (qn->kind) {
     case QNUM_I64:
         return g_strdup_printf("%" PRId64, qn->u.i64);
     case QNUM_U64:
         return g_strdup_printf("%" PRIu64, qn->u.u64);
     case QNUM_DOUBLE:
-        /* FIXME: snprintf() is locale dependent; but JSON requires
+        /* FIXME: g_strdup_printf() is locale dependent; but JSON requires
          * numbers to be formatted as if in the C locale. Dependence
          * on C locale is a pervasive issue in QEMU. */
         /* FIXME: This risks printing Inf or NaN, which are not valid
          * JSON values. */
-        /* FIXME: the default precision of 6 for %f often causes
-         * rounding errors; we should be using DBL_DECIMAL_DIG (17),
-         * and only rounding to a shorter number if the result would
-         * still produce the same floating point value.  */
-        buffer = g_strdup_printf("%f" , qn->u.dbl);
-        len = strlen(buffer);
-        while (len > 0 && buffer[len - 1] == '0') {
-            len--;
-        }
-
-        if (len && buffer[len - 1] == '.') {
-            buffer[len - 1] = 0;
-        } else {
-            buffer[len] = 0;
-        }
-
-        return buffer;
+        /* 17 digits suffice for IEEE double */
+        return g_strdup_printf("%.17g", qn->u.dbl);
     }
 
     assert(0);
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 98515b1fd6..ca8fb816e9 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -882,10 +882,10 @@ static void float_number(void)
     } test_cases[] = {
         { "32.43", 32.43 },
         { "0.222", 0.222 },
-        { "-32.12313", -32.12313 },
-        { "-32.20e-10", -32.20e-10, "-0" /* BUG */ },
-        { "18446744073709551616", 0x1p64 },
-        { "-9223372036854775809", -0x1p63, "-9223372036854775808" },
+        { "-32.12313", -32.12313, "-32.123130000000003" },
+        { "-32.20e-10", -32.20e-10, "-3.22e-09" },
+        { "18446744073709551616", 0x1p64, "1.8446744073709552e+19" },
+        { "-9223372036854775809", -0x1p63, "-9.2233720368547758e+18" },
         {},
     };
     int i;
diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index a73809d021..b85fca2302 100644
--- a/tests/check-qnum.c
+++ b/tests/check-qnum.c
@@ -147,13 +147,13 @@ static void qnum_to_string_test(void)
 
     qn = qnum_from_double(0.42);
     tmp = qnum_to_string(qn);
-    g_assert_cmpstr(tmp, ==, "0.42");
+    g_assert_cmpstr(tmp, ==, "0.41999999999999998");
     g_free(tmp);
     qobject_unref(qn);
 
     qn = qnum_from_double(2.718281828459045);
     tmp = qnum_to_string(qn);
-    g_assert_cmpstr(tmp, ==, "2.718282"); /* BUG */
+    g_assert_cmpstr(tmp, ==, "2.7182818284590451");
     g_free(tmp);
     qobject_unref(qn);
 }
-- 
2.26.2