On Fri, May 10, 2013 at 11:17:17AM -0400, Luiz Capitulino wrote:
> On Thu, 9 May 2013 21:20:58 -0500
> Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
>
> > Currently our JSON parser assumes that numbers lacking a mantissa are
> > integers and attempts to store them as QInt/int64 values. This breaks in
> > the case where the number overflows/underflows int64 values (which is
> > still valid JSON)
>
> Anthony wanted to fix this by moving to another wire format :)
>
> But, how this patch related to this series?
In v1 Laszlo pointed out that the QFloat tests I added in
test-visitor-serialization were actually non-functional, and those tests were
based on pre-existing code. So I added a patch to fix the pre-existing
code as a pre-cursor to the new unit tests based on it. But fixing that code
exposed the json-parser bug, so I added that fix as a precursor to the
precursor :)
Same with mem leak fixes, etc, to ensure the tests were functional and
leak-free within this series.
>
> >
> > Fix this by detecting such cases and using a QFloat to store the value
> > instead.
> >
> > Signed-off-by: Michael Roth <mdr...@linux.vnet.ibm.com>
> > ---
> > qobject/json-parser.c | 26 +++++++++++++++++++++++---
> > 1 file changed, 23 insertions(+), 3 deletions(-)
> >
> > diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> > index 05279c1..4d14e71 100644
> > --- a/qobject/json-parser.c
> > +++ b/qobject/json-parser.c
> > @@ -640,9 +640,29 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> > case JSON_STRING:
> > obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
> > break;
> > - case JSON_INTEGER:
> > - obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL,
> > 10)));
> > - break;
> > + case JSON_INTEGER: {
> > + /* A possibility exists that this is a whole-valued float where the
> > + * mantissa was left out due to being 0 (.0). It's not a big deal
> > to
> > + * treat these as ints in the parser, so long as users of the
> > + * resulting QObject know to expect a QInt in place of a QFloat in
> > + * cases like these.
> > + *
> > + * However, in some cases these values will overflow/underflow a
> > + * QInt/int64 container, thus we should assume these are to be
> > handled
> > + * as QFloats/doubles rather than silently changing their values.
> > + *
> > + * strtoll() indicates these instances by setting errno to ERANGE
> > + */
> > + int64_t value;
> > +
> > + errno = 0; /* strtoll doesn't set errno on success */
> > + value = strtoll(token_get_value(token), NULL, 10);
> > + if (errno != ERANGE) {
> > + obj = QOBJECT(qint_from_int(value));
> > + break;
> > + }
> > + /* fall through to JSON_FLOAT */
> > + }
> > case JSON_FLOAT:
> > /* FIXME dependent on locale */
> > obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token),
> > NULL)));
>
