[Qemu-devel] [PATCH 4/4] qobject: Output valid JSON for non-finite numbers

[Eric Blake]

It's better to give downstream clients a valid JSON string,
even if they are semantically expecting a number, than it is
to give them a bare keyword extension that can cause a
lexical error.

Of course, as long as we don't recognize (certain) strings as valid
numbers during a conversion to QObject, this means our extension
of accepting bare keywords for non-finite numbers cannot undergo
a round trip (once converted into a string, we never get back to
a QFloat).  However, non-finite input is rare enough that it's
not worth bothering with at the moment.

Signed-off-by: Eric Blake <address@hidden>
---
 qobject/qjson.c     | 15 ++++++++++++---
 tests/check-qjson.c | 11 +++++------
 docs/qmp-spec.txt   |  2 +-
 3 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/qobject/qjson.c b/qobject/qjson.c
index ef160d2..465b080 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -12,6 +12,7 @@
  */

 #include "qemu/osdep.h"
+#include <math.h>
 #include "qapi/qmp/json-lexer.h"
 #include "qapi/qmp/json-parser.h"
 #include "qapi/qmp/json-streamer.h"
@@ -236,19 +237,27 @@ static void to_json(const QObject *obj, QString *str, int 
pretty, int indent)
     }
     case QTYPE_QFLOAT: {
         QFloat *val = qobject_to_qfloat(obj);
+        double d = qfloat_get_double(val);
         char buffer[1024];
         int len;

+        if (!isfinite(d)) {
+            /* Always output valid JSON - a semantic error due to a
+             * string where a number was expected is better than a
+             * lexical error from a strict parser */
+            snprintf(buffer, sizeof(buffer), "\"%f\"", d);
+            qstring_append(str, buffer);
+            break;
+        }
+
         /* FIXME: snprintf() 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.  */
-        len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
+        len = snprintf(buffer, sizeof(buffer), "%f", d);
         while (len > 0 && buffer[len - 1] == '0') {
             len--;
         }
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 95adf64..b0a9178 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -974,12 +974,12 @@ static void non_finite_number(void)
         double decoded;
         const char *canonical;
     } test_cases[] = {
-        { "nan", NAN },
-        { "NaN", NAN, "nan" },
+        { "nan", NAN, "\"nan\"" },
+        { "NaN", NAN, "\"nan\"" },
         /* Not all libc implementations can round-trip '-nan' */
         /* We do not support forms like 'NAN(0)' */
-        { "inf", INFINITY },
-        { "-INFINITY", -INFINITY, "-inf" },
+        { "inf", INFINITY, "\"inf\"" },
+        { "-INFINITY", -INFINITY, "\"-inf\"" },
         { },
     };

@@ -1002,8 +1002,7 @@ static void non_finite_number(void)
         }

         str = qobject_to_json(obj);
-        g_assert_cmpstr(qstring_get_str(str), ==,
-                        test_cases[i].canonical ?: test_cases[i].encoded);
+        g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].canonical);
         QDECREF(str);
         QDECREF(qfloat);
     }
diff --git a/docs/qmp-spec.txt b/docs/qmp-spec.txt
index bfba431..75c68d9 100644
--- a/docs/qmp-spec.txt
+++ b/docs/qmp-spec.txt
@@ -53,7 +53,7 @@ double quoting on output.

 As an extension, the server understands case-insensitive forms of
 non-finite numbers, such as "NaN", "Inf", and "-infinity" (but not
-"NaN(...)").
+"NaN(...)"); however, such numbers are output as a json-string.

 2.1 General Definitions
 -----------------------
-- 
2.5.5