From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52027) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UappD-0004pB-NQ for qemu-devel@nongnu.org; Fri, 10 May 2013 12:05:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UappC-0004lD-Dw for qemu-devel@nongnu.org; Fri, 10 May 2013 12:05:39 -0400 Received: from mail-ie0-x22a.google.com ([2607:f8b0:4001:c03::22a]:42177) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UappC-0004l4-7Y for qemu-devel@nongnu.org; Fri, 10 May 2013 12:05:38 -0400 Received: by mail-ie0-f170.google.com with SMTP id aq17so8440852iec.1 for ; Fri, 10 May 2013 09:05:37 -0700 (PDT) Sender: fluxion Date: Fri, 10 May 2013 11:00:50 -0500 From: mdroth Message-ID: <20130510160050.GG13213@vm> References: <1368152462-13219-1-git-send-email-mdroth@linux.vnet.ibm.com> <1368152462-13219-7-git-send-email-mdroth@linux.vnet.ibm.com> <20130510111717.4a9449a9@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20130510111717.4a9449a9@redhat.com> Subject: Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: akong@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org On Fri, May 10, 2013 at 11:17:17AM -0400, Luiz Capitulino wrote: > On Thu, 9 May 2013 21:20:58 -0500 > Michael Roth 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 > > --- > > 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))); >