From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52095) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bxhEx-0002Bz-P4 for qemu-devel@nongnu.org; Fri, 21 Oct 2016 17:20:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bxhEu-00025Z-HO for qemu-devel@nongnu.org; Fri, 21 Oct 2016 17:20:35 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51824) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bxhEu-00025Q-49 for qemu-devel@nongnu.org; Fri, 21 Oct 2016 17:20:32 -0400 References: <1476782267-2602-1-git-send-email-ptoscano@redhat.com> <3321636.HNiH4nMqTU@thyrus.usersys.redhat.com> From: Eric Blake Message-ID: <18fc0471-2259-b0bd-3e0c-2cd7ea3bc394@redhat.com> Date: Fri, 21 Oct 2016 16:20:30 -0500 MIME-Version: 1.0 In-Reply-To: <3321636.HNiH4nMqTU@thyrus.usersys.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="NdbXBjQbiUQHWh2Wv6fbsQw4G7Jcbn0JA" Subject: Re: [Qemu-devel] [PATCH] qapi: fix memory leak in QmpOutputVisitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Pino Toscano Cc: qemu-devel@nongnu.org, armbru@redhat.com, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --NdbXBjQbiUQHWh2Wv6fbsQw4G7Jcbn0JA From: Eric Blake To: Pino Toscano Cc: qemu-devel@nongnu.org, armbru@redhat.com, mdroth@linux.vnet.ibm.com Message-ID: <18fc0471-2259-b0bd-3e0c-2cd7ea3bc394@redhat.com> Subject: Re: [Qemu-devel] [PATCH] qapi: fix memory leak in QmpOutputVisitor References: <1476782267-2602-1-git-send-email-ptoscano@redhat.com> <3321636.HNiH4nMqTU@thyrus.usersys.redhat.com> In-Reply-To: <3321636.HNiH4nMqTU@thyrus.usersys.redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/18/2016 06:22 AM, Pino Toscano wrote: > On Tuesday, 18 October 2016 06:13:30 CEST Eric Blake wrote: >> On 10/18/2016 04:17 AM, Pino Toscano wrote: >>> qmp_output_start_struct() and qmp_output_start_list() create a new >>> QObject (QDict, QList) and push it to the stack of the QmpOutputVisit= or, >>> where it is saved as 'value'. When freeing the iterator in >>> qmp_output_free(), these values are never freed properly. >> >> Do any of the tests (perhaps run under valgrind) show this leak? If no= t, >> maybe we should enhance their coverage. >=20 > Running a simple `qemu-img info file.qcow2` under valgrind was enough > for me to show the leak. I'm still not reproducing it. :( >=20 > In this case, another simple fix is needed to fully fix the leak: > http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg04023.html In fact, isn't that fix alone enough to fix the leak? The more I think about this patch (and the thread on v2), the more I think it is too prone to double-freeing things. >>> +++ b/qapi/qmp-output-visitor.c >>> @@ -220,6 +220,7 @@ static void qmp_output_free(Visitor *v) >>> while (!QSLIST_EMPTY(&qov->stack)) { >>> e =3D QSLIST_FIRST(&qov->stack); >>> QSLIST_REMOVE_HEAD(&qov->stack, node); >>> + qobject_decref(e->value); >>> g_free(e); >>> } >>> =20 >>> >> >> >=20 >=20 --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --NdbXBjQbiUQHWh2Wv6fbsQw4G7Jcbn0JA 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/ iQEcBAEBCAAGBQJYCoaeAAoJEKeha0olJ0Nqi2IH/2ODDJ8tJ7kNeEI3LnaG/8fK UTAzBUK59HQ2yvtbGTrK4vgWKZP8j6eL+9v99fM057rJLVAWekXBKC2pH8tN0rSj bwO/bRbOuWoiL3EGTMCaujp6z1s3SY5/cP6coIcq/erXqdTUwm2Y6ppUVP5P43Md KvwvWOCs1n7elcbBxTNtXSZRbVF/59ffQuTK5ItMfQpZ+m4kKVPTeQunzzXGEHe3 DUOdeTWgPkiL/Eske9A4HPrxPGJYaETS0vzlUbCtsHnyz5ZCa86KJaXiQUB5tFZs AcpiJmU2EAybLUclEurVoSHHw5qFtNz+PoNOXGUaY6ODfosh1+DSo1rDpNUG5y4= =heUE -----END PGP SIGNATURE----- --NdbXBjQbiUQHWh2Wv6fbsQw4G7Jcbn0JA--