From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UamMH-0003bl-Nt for qemu-devel@nongnu.org; Fri, 10 May 2013 08:23:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UamMG-0001RB-9K for qemu-devel@nongnu.org; Fri, 10 May 2013 08:23:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:4714) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UamMG-0001PY-1n for qemu-devel@nongnu.org; Fri, 10 May 2013 08:23:32 -0400 Message-ID: <518CE683.4000600@redhat.com> Date: Fri, 10 May 2013 06:22:27 -0600 From: Eric Blake MIME-Version: 1.0 References: <1368152462-13219-1-git-send-email-mdroth@linux.vnet.ibm.com> <1368152462-13219-7-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1368152462-13219-7-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2IWBEONPPGNBXBOEVAOLE" 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: Michael Roth Cc: akong@redhat.com, lersek@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2IWBEONPPGNBXBOEVAOLE Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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 i= n > the case where the number overflows/underflows int64 values (which is > still valid JSON) >=20 > Fix this by detecting such cases and using a QFloat to store the value > instead. >=20 > Signed-off-by: Michael Roth > --- > 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2IWBEONPPGNBXBOEVAOLE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRjOaDAAoJEKeha0olJ0NqnuUH/3LGOtMnFhLH+pX+xdhcacV+ rxb/JqDkxzrs6JW56KrCymDT1yEycA6CiB5rb6fxlJ4mJZQsMpd1PUWhO+tExMsM ZY5JEQRFC6mGl29K+TX8HSqeZRlAaDqRiQdLqo1YZxD33F73g0KhVmxTy44WA6dd 8/tdPWe8Adg55P7Yw/z62lID6AnA3O5VV9i+meoJ7uj3nAPKFtEnwFrY6xtS3DyJ GFd5umwm+Lo0Tupvj517sl6Mid6RVMTKwh6aksxVmW5gAMvob8B7qlBxyU97ilyI NYAzCljnY4Ma0NV6DHcMBtZBWDyJEQUyVpRaY7w3V0WgP0K8rrHT3WUHYIJElrg= =rd1V -----END PGP SIGNATURE----- ------enig2IWBEONPPGNBXBOEVAOLE--