Eric Blake <ebl...@redhat.com> writes: > On 06/03/2016 03:02 AM, Markus Armbruster wrote: > >>>> Suggest: >>>> >>>> * Return 0 if the number is finite, as required by RFC 7159, else -1. >>>> >>>> The return value makes some sense only for symmetry with >>>> qstring_append_json_string(). Without that, I'd ask you to keep this >>>> function simple. Callers could just as easily test isfinite() >>>> themselves. >>> >>> I'm actually thinking of modifying this, given the recent thread >>> pointing out that libvirt chokes hard on JSON extensions: >>> >>> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg04398.html >>> >>> That is, for symmetry with qstring_append_json_string(), I'm thinking of >>> changing NaN to 0 and Inf to DBL_MAX, and always outputting a finite >>> value, in addition to returning -1 to inform the caller that a >>> substitution was made, so that the output is always strict JSON. >> >> Mapping infinities to DBL_MIN and DBL_MAX is debatable, but mapping NaN >> to zero is outright wrong. > > How about this alternative: > > Finite values remain numbers: > "number":1 > > But non-finite values are output as strings, so that our output is > always valid JSON - the recipient may not be expecting a string in place > of a number, but at least should be able to parse the output rather than > choking hard. > "number":"nan" > > The return value -1 then indicates that a stringized replacement was > used, so that any later patch can use a strict flag on whether to allow > the replacement output or assert.
I dislike this a lot less than mapping non-finite numbers to finite ones. I still dislike it, because it defeats fitting a schema to QMP: instead of the true JSON type "number", we'd need the sum type of "number" and "string", which this really isn't: only a few special strings are valid, and they're not actually strings. If the schema language can do sum types, we'd even be stuck with their common super-type. The most practical solution isn't always a likable one, though. >> If we decide QMP should stick to JSON here and avoid non-finite numbers, >> we need to treat an attempt to emit a non-finite number as a bug: >> assert(isfinite(...)). Making sure nothing ever attempts to emit such >> numbers will be tedious. >> >> If we decide QMP should remain as it is, we need to document non-finite >> numbers among its JSON extensions. We should also fix our QMP parsers >> to accept non-finite numbers then. Including the one in libvirt. >> Attempts to emit non-finite numbers then *may* be bugs. Really no >> different than finite numbers outside their intended range, such as a >> negative size. Catching these bugs is of course also tedious. The >> difference is that they manifest in QMP as semantic instead of lexical >> errors. Lexical errors are the worst to handle gracefully. > > I may still try to tackle fixing the QMP parser to accept NaN and > infinity on input (since it's hand-written, we at least have control > over that) Making json-lexer.c recognize infinities and NaNs in strtod() syntax shouldn't be hard. I'd omit nan(n-char-sequence-opt), because its semantics are implementation defined. I'd also omit all spellings other than inf and nan. That leaves inf, +inf, -inf, nan, +nan, -nan. > - it will certainly be easier than getting libvirt to parse > non-finite numbers (libvirt uses libyajl, and my emails to the yajl > mailing list have gone unanswered, making me think the project is not > very vibrant and thus not very patchable). Nobody likes to carry downstream patches, but an unresponsive upstream may leave you no choice. > But with my proposal of > producing a stringized non-finite value, we at least convert lexical > into semantic errors, which I agree with your assessment is a nicer way > of dealing with it. > > Of course, a policy change of outputting stringized non-finite numbers > should be separate from refactoring patches that just move functions around. Yes.