[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 01/37] qobject: Document more shortcomings in
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v9 01/37] qobject: Document more shortcomings in our number handling |
Date: |
Wed, 20 Jan 2016 10:02:25 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> We've already documented that our JSON parsing is locale dependent;
> but we should also document that our JSON output has the same
> problem. Additionally, JSON requires finite values (you have to
> upgrade to JSON5 to get support for Inf or NaN), and our output
> risks truncating floating point numbers to the point of losing
> significant precision.
>
> Sadly, this series is not going to be the one that addresses these
> problems.
>
> Fix some trailing whitespace I noticed in the vicinity.
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Marc-André Lureau <address@hidden>
>
> ---
> v9: no change
> v8: no change
> v7: new patch
> ---
> qobject/json-parser.c | 4 +++-
> qobject/qjson.c | 8 +++++++-
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 3c5d35d..6ab98a7 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -1,5 +1,5 @@
> /*
> - * JSON Parser
> + * JSON Parser
> *
> * Copyright IBM, Corp. 2009
> *
> @@ -519,6 +519,8 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> }
> case JSON_FLOAT:
> /* FIXME dependent on locale */
> + /* FIXME our lexer matches RFC7159 in forbidding Inf or NaN,
For what it's worth, the RFC spells this "RFC 7159".
> + * but those might be useful extensions beyond JSON */
> return QOBJECT(qfloat_from_double(strtod(token->str, NULL)));
> default:
> abort();
> diff --git a/qobject/qjson.c b/qobject/qjson.c
> index a3e6a7c..41d9d65 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qjson.c
> @@ -237,6 +237,12 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> char buffer[1024];
> int len;
>
> + /* FIXME: snprintf() is locale dependent; but JSON requires
> + * numbers to be formatted as if in the C locale. */
The new FIXME matches the existing one in parse_literal(), visible
above.
However, dependence on C locale is a pervasive issue in QEMU. These two
comments could give readers the idea that it's an issue just here.
Suggest to add something like "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 %f may be insufficient to
> + * tell this number apart from others. */
Yup.
The easy way to avoid loss of precision is %a, but of course that's way
too much sophistication for JSON.
Avoiding loss of precision with a decimal format is non-trivial; see
Steele, Jr., Guy L., and White, Jon L. How to print floating-point
numbers accurately, SIGPLAN ’90, and later improvements collected here:
http://stackoverflow.com/questions/7153979/algorithm-to-convert-an-ieee-754-double-to-a-string
> len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
> while (len > 0 && buffer[len - 1] == '0') {
> len--;
> @@ -247,7 +253,7 @@ static void to_json(const QObject *obj, QString *str, int
> pretty, int indent)
> } else {
> buffer[len] = 0;
> }
> -
> +
> qstring_append(str, buffer);
> break;
> }
[Qemu-devel] [PATCH v9 13/37] qom: Use typedef for Visitor, Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 02/37] qapi: Avoid use of misnamed DO_UPCAST(), Eric Blake, 2016/01/19
[Qemu-devel] [PATCH v9 12/37] qapi: Don't cast Enum* to int*, Eric Blake, 2016/01/19