Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > --- > 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; > }