From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60168) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c9YJK-0000aY-Ac for qemu-devel@nongnu.org; Wed, 23 Nov 2016 09:14:07 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c9YJH-0003q3-1Z for qemu-devel@nongnu.org; Wed, 23 Nov 2016 09:14:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59766) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c9YJG-0003pN-OZ for qemu-devel@nongnu.org; Wed, 23 Nov 2016 09:14:02 -0500 References: <1479874588-1969-1-git-send-email-eblake@redhat.com> <1479874588-1969-3-git-send-email-eblake@redhat.com> <87r362pa05.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <406c8a0c-1d4a-bbd3-a2de-6fb7fb9f4702@redhat.com> Date: Wed, 23 Nov 2016 08:14:00 -0600 MIME-Version: 1.0 In-Reply-To: <87r362pa05.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Xo8vW2jeSb6sab0iS8WPjRQo9JeD8eBvF" Subject: Re: [Qemu-devel] [PATCH 2/3] test-qga: Avoid qobject_from_jsonf("%"PRId64) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, programmingkidx@gmail.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Xo8vW2jeSb6sab0iS8WPjRQo9JeD8eBvF From: Eric Blake To: Markus Armbruster Cc: qemu-devel@nongnu.org, programmingkidx@gmail.com, pbonzini@redhat.com Message-ID: <406c8a0c-1d4a-bbd3-a2de-6fb7fb9f4702@redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/3] test-qga: Avoid qobject_from_jsonf("%"PRId64) References: <1479874588-1969-1-git-send-email-eblake@redhat.com> <1479874588-1969-3-git-send-email-eblake@redhat.com> <87r362pa05.fsf@dusky.pond.sub.org> In-Reply-To: <87r362pa05.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/23/2016 08:05 AM, Markus Armbruster wrote: > Same problem as in the previous patch, but here you replace it by > g_strdup_printf(), where the previous patch replaced it by manual > QObject construction, >=20 > Manual QObject construction tends to be less readable. Are there things we can do to make it more readable to the point where it would be tolerable in the situations where it is needed? One of the patches on my dynamic-JSON removal series adds a new: qdict_put_int(dict, "key", 1); which is a lot more legible than: qdict_put(dict, "key", qint_from_int(1)); >=20 > g_strdup_printf() doesn't have that problem, but it has a more serious > one: escaping for JSON is no longer below the hood. >=20 > Since the string gets passed to qmp_fd(), we additionally need to escap= e > '%'. Worse, the escaping of %s differs between the two (in printf, %s just concatenates strings, in dynamic JSON, it adds outer "" and escapes inner " into \"). >=20 > Interfaces that require callers to escape almost inevitably result in > bugs if experience is any guide. Safer, less low level interfaces are > preferable. >=20 > Nothing actually needs escaping here, so your code isn't wrong. It's > just a bad example. >=20 > You've pointed out that the file is chock-full of bad examples already,= > so one more won't make a difference. Point taken regarding the > immediate fix. But I doubt it a sane strategy for replacing _jsonf(). >=20 Well, until I post my conversion series that eliminates _json[fv](), we don't have any hard numbers on how many bad examples remain, or whether the cleanup looks worth it. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --Xo8vW2jeSb6sab0iS8WPjRQo9JeD8eBvF Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJYNaQoAAoJEKeha0olJ0NqmekH/07un79GepM/kSeQA//bQLqo MT5QwURcmH8cHfN55QHhPlEOCny14rf+j878/iRoPMb2upi3PLWV7uV6v0nx7eBF Gs+GC7oTlpcXBDBbCaF9LyT4xsAVm4o+ML5hmbpkZlcbGYMDxAxzFLIsFkVxNvDB y6JzrKCFJ41HNdDbOZoZ0ZVIuvR9NP0CGORcvUuTv7dNjO+ShyRfnA5vgVV1l3Yd kVoPht0HvIyGHR6lrnv1Zljq6+MRuZB2xsrDvv55QqQX50BaTWuD/+55boi4yHbd tDbvmxEzy/VSMrDa+BG66erjpGxDGlO40BAr3EseXeiMgXwuwasArmsvK51CRK4= =oqwU -----END PGP SIGNATURE----- --Xo8vW2jeSb6sab0iS8WPjRQo9JeD8eBvF--