Re: [Qemu-devel] [PATCH v9 01/37] qobject: Document more shortcomings in our number handling

[Daniel P. Berrange]

On Thu, Jan 21, 2016 at 07:21:49AM +0100, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
> > On 01/20/2016 02:02 AM, Markus Armbruster wrote:
> >
> >>> @@ -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".
> >
> > Looks like we use space more often than not, but that we're
> > inconsistent.  For example:
> >
> > slirp/tcp.h: * Per RFC 793, September, 1981.
> > slirp/tcp.h: * Per RFC793, September, 1981.
> >
> > Will fix if I need to respin, otherwise I assume you can do it.
> 
> Okay.
> 
> >>> +        /* 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."
> >
> > Good idea.
> >
> >> 
> >>> +        /* 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
> >
> > Ah, memories.  I read and implemented that paper when working on the
> > jikes compiler for the Java language back in the late nineties, as it is
> > the Java language specification which had the very neat property of
> > requiring the shortest decimal string that will unambiguously round back
> > to the same floating point pattern.
> >
> > One alternative is to always output a guaranteed unambiguous decimal
> > string (although not necessarily the shortest), by using %.17f, using
> > <float.h> DBL_DECIMAL_DIG.  (Note that DBL_DIG of 15 is NOT sufficient -
> > it is the lower limit that says that a decimal->float->decimal will not
> > change the decimal; but we want the converse where a
> > float->decimal->float will not change the float.  There are stretches of
> > numbers where the pigeonhole principle applies; you can think of it this
> > way: there is no way to map all possible 2^10 (1024) binary values
> > inside 2^3 (1000) decimal digits without at least 24 of them needing one
> > more decimal digit.  But by the same arguments, DBL_DECIMAL_DIG is an
> > upper limit and usually more than you need.)
> >
> > So, the question is whether we want to always output 17 digits, or
> > whether we want to do the poor-man's truncation scheme (easy to
> > understand, but not optimal use of the processor), or go all the way to
> > the algorithm of that paper (faster but lots harder to understand).  For
> > reference, here's the poor-man's algorithm in pseudocode:
> 
> I don't think we want to implement floating-point formatting ourselves.

FWIW in libvirt we use per-thread locale switching, so that we can flip
into the C locale temporarily when formatting doubles. If those are not
available we have a bit of a nasty hack to post-process the locale
dependant string back into what we (hope) is C-locale formatting.
We don't do anything aobut the Inf/NaN problem though.

http://libvirt.org/git/?p=libvirt.git;a=blob;f=src/util/virutil.c;h=bb9604a0c1ffb9c99e454e84878a8c376f773046;hb=HEAD#l454

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|