[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values |
Date: |
Fri, 10 May 2013 06:22:27 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5 |
On 05/09/2013 08:20 PM, 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)
>
> Fix this by detecting such cases and using a QFloat to store the value
> instead.
>
> Signed-off-by: Michael Roth <address@hidden>
> ---
> qobject/json-parser.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
This changes the error message handed back to QMP clients, and possibly
exposes problems in other qemu code that receives the result of json
parses. Previously, for an 'int' argument, if you passed in a too-large
number, you got an error that the argument was too large for int. Now,
the number is accepted as a double; are we guaranteed that in a context
that expects a qint, when that code is now handed a qfloat (a case which
was previously impossible because qint_from_int protected it), that the
code will still behave correctly?
At any rate, libvirt already checks that all numbers that fall outside
the range of int64_t are never passed over qmp when passing an int
argument (and yes, this is annoying, in that large 64-bit unsigned
numbers have to be passed as negative numbers, rather than exceeding
INT64_MAX), so libvirt should not be triggering this newly exposed code
path. But even if libvirt doesn't plan on triggering it, I'd still feel
better if your commit message documented evidence of testing what
happens in this case. For example, compare what
{"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
does before and after this patch.
> + /* fall through to JSON_FLOAT */
> + }
> case JSON_FLOAT:
> /* FIXME dependent on locale */
You know, strtoll() also is dependent on locale (that is, non-C locale
can accept strings that the C locale rejects), if you want to repeat
this comment earlier for case JSON_INTEGER.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support, (continued)
- [Qemu-devel] [PATCH 02/10] qapi: qapi-visit.py, fix list handling for union types, Michael Roth, 2013/05/09
- [Qemu-devel] [PATCH 03/10] qapi: qapi-visit.py, native list support, Michael Roth, 2013/05/09
- [Qemu-devel] [PATCH 04/10] qapi: enable generation of native list code, Michael Roth, 2013/05/09
- [Qemu-devel] [PATCH 05/10] qapi: fix leak in unit tests, Michael Roth, 2013/05/09
- [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values, Michael Roth, 2013/05/09
- Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values, Luiz Capitulino, 2013/05/10
[Qemu-devel] [PATCH 07/10] qapi: fix visitor serialization tests for numbers/doubles, Michael Roth, 2013/05/09
[Qemu-devel] [PATCH 08/10] qapi: add native list coverage for visitor serialization tests, Michael Roth, 2013/05/09
[Qemu-devel] [PATCH 09/10] qapi: add native list coverage for QMP output visitor tests, Michael Roth, 2013/05/09
[Qemu-devel] [PATCH 10/10] qapi: add native list coverage for QMP input visitor tests, Michael Roth, 2013/05/09