From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48815) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLARb-0007W8-JR for qemu-devel@nongnu.org; Thu, 07 Jul 2016 10:38:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bLARX-0004Vc-Dj for qemu-devel@nongnu.org; Thu, 07 Jul 2016 10:38:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43715) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bLARX-0004VW-5T for qemu-devel@nongnu.org; Thu, 07 Jul 2016 10:38:19 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AC8BD3F724 for ; Thu, 7 Jul 2016 14:38:18 +0000 (UTC) References: <1467809017-25023-1-git-send-email-pbonzini@redhat.com> <1467809017-25023-2-git-send-email-pbonzini@redhat.com> <878txdsuss.fsf@dusky.pond.sub.org> From: Eric Blake Message-ID: <577E6928.1080904@redhat.com> Date: Thu, 7 Jul 2016 08:37:28 -0600 MIME-Version: 1.0 In-Reply-To: <878txdsuss.fsf@dusky.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="D4tPuBt2k8dDuUMND6UD4AR4R8TRk1PMe" Subject: Re: [Qemu-devel] [PATCH] qapi: change QmpInputVisitor to QSLIST List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Paolo Bonzini Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --D4tPuBt2k8dDuUMND6UD4AR4R8TRk1PMe From: Eric Blake To: Markus Armbruster , Paolo Bonzini Cc: qemu-devel@nongnu.org Message-ID: <577E6928.1080904@redhat.com> Subject: Re: [Qemu-devel] [PATCH] qapi: change QmpInputVisitor to QSLIST References: <1467809017-25023-1-git-send-email-pbonzini@redhat.com> <1467809017-25023-2-git-send-email-pbonzini@redhat.com> <878txdsuss.fsf@dusky.pond.sub.org> In-Reply-To: <878txdsuss.fsf@dusky.pond.sub.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/07/2016 02:42 AM, Markus Armbruster wrote: > Needs a rebase. The other one, too. >=20 > Paolo Bonzini writes: >=20 >> This saves a lot of memory compared to a statically-sized array. >> >> Signed-off-by: Paolo Bonzini >=20 > Saves 24KiB - d*32B. That's "a lot" for an Atari ST or something, or i= f > you're juggling hundreds of these things at the same time. >=20 > Allocating 24KiB and using only the first d*24 bytes is simpler and > generally faster than allocating d chunks of 32B each. But I won't > argue, as I'm pretty confident that neither memory nor CPU use of this > code matter. >=20 > Another reason not to argue: commit e38ac96 already added a per-depth > allocation. >=20 > So all I ask for here is s/a lot of memory/some memory/. >=20 > The patch makes QmpInputVisitor more similar to QmpOutputVisitor, which= > is nice. >=20 Yes, this second effect is good enough reason for the patch, regardless of the merits of any memory savings in the first effect. >> assert(obj); >> - if (qiv->nb_stack >=3D QIV_STACK_SIZE) { >> - error_setg(errp, "An internal buffer overran"); >> - return NULL; >> - } >=20 > Removing the arbitrary limit is aesthetically pleasing. But can > untrusted input make us allocate unbounded amounts of memory now? Elsewhere in the thread, we mentioned that the input visitor never visits more than what an existing QObject already has in memory. Calling that out in the commit message might have been nice (but v2 did not do that). >=20 >> - >> tos->obj =3D obj; >> - assert(!tos->h); >> - assert(!tos->entry); >=20 > Why do you delete these two? >=20 Because pre-patch, these fields are inherited in the state left on the stack from any earlier visits (we are asserting that other branches of the visit left things in a sane state), but post-patch, we are malloc0()ing each tos fresh (rather than reusing). --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --D4tPuBt2k8dDuUMND6UD4AR4R8TRk1PMe 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/ iQEcBAEBCAAGBQJXfmkoAAoJEKeha0olJ0Nqdb4IAI/x/5kXNrTNh4Wz9c5O4LTR i5untth9VPGWUKFP604hG3CqOsfTAKlrev7OnpLuh47JYY3MKMjnyjFCkHmp4a7g 9FrCX98PqwNcTZejiPyTlfM6lDEm9n0xUhwJLnrDd7+Mj3VGTBJkI1liIL++Z5LF 1uojW3XfM2BtFuFBvCaw35BryJAjvoEloai7GFdANpgHjY0MGAXt7MMdgMnZ2vBa B3MaPmDXupSUxY4TLcIgxEL+VW8gSBl01uB4sv+CQYpV4RvZalntam+Qzws4w935 UvsOQswfzR+RUWdi2DjNdWqwjrMetQE0VX2CuJS6KcSgGwgK/PbmYWR0ZgIfJGc= =Vvzf -----END PGP SIGNATURE----- --D4tPuBt2k8dDuUMND6UD4AR4R8TRk1PMe--