From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46710) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zalcr-0002cE-EJ for qemu-devel@nongnu.org; Sat, 12 Sep 2015 10:17:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zalcm-0008D1-Qt for qemu-devel@nongnu.org; Sat, 12 Sep 2015 10:17:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53996) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zalcm-0008C7-JZ for qemu-devel@nongnu.org; Sat, 12 Sep 2015 10:17:52 -0400 References: <1441999089-28960-1-git-send-email-armbru@redhat.com> <1441999089-28960-21-git-send-email-armbru@redhat.com> <55F33D06.1090103@redhat.com> <55F3411B.5070905@redhat.com> <87a8ssf47y.fsf@blackfin.pond.sub.org> <55F41788.6030008@redhat.com> <87613fd8wq.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <55F43409.7060906@redhat.com> Date: Sat, 12 Sep 2015 08:17:45 -0600 MIME-Version: 1.0 In-Reply-To: <87613fd8wq.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="McqAB8WelXNIERSeijLpPoBaTJonavot0" Subject: Re: [Qemu-devel] [PATCH v6 20/26] qapi: Fix output visitor to return qnull() instead of NULL List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --McqAB8WelXNIERSeijLpPoBaTJonavot0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/12/2015 08:11 AM, Markus Armbruster wrote: >> Indeed. Without reading further, we're both shooting in the dark for >> something that makes tests pass, but without being a clean interface. >> >> How about this: go ahead with your series as proposed, with the squash= >> hunk to tests/ to avoid the leak in the testsuite, but leaving the lea= k >> in qmp_output_get_object(), and we address the leak in a followup patc= h. I've given a couple more responses with my analysis, but up to you how much you want to take now or defer to later. >=20 > I'll add a FIXME explaining the reference counting bug, and the wider > problem. >=20 > What exactly do you want changed in tests? Definitely add the qobject_decref(arg). But the g_assert(refcnt...) should not be added unless we go with a solution that doesn't leak, and instead should be replaced with a FIXME, if you don't want my followup mails now. >> The followup patch(es) will then have to include some contract >> documentation, so that we track what we learned while investigating th= e >> code, and so that the next reader has more than just code to start fro= m. >=20 > It's time to retrofit visitors with a proper contract. >=20 > Retrofitting a contract is much harder than starting with one, but we > got to play the hand we've been dealt. I've already started that work in some of my pending patches: https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02597.html but it could indeed use further documentation in each visitor implementation. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --McqAB8WelXNIERSeijLpPoBaTJonavot0 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/ iQEcBAEBCAAGBQJV9DQJAAoJEKeha0olJ0NqF9EH/1XN0zOF7NxzRYa/FXCXW8ar 4uhDKQCi0MCwyTSHyQs3iyFHM4oQczftl/g1Z5stlKSziBH6yBXLYL2XTr5EL7nR Vh9ZgPwx78qtLT4UHoteGizX9Ffpyme8Aw3Qb1qzTtZfJ8g/IGLPlBCdoIosOnoM J3HC4VPObMFxJ8pnPQMNomgnqi5MidXNxq6fwezEizHqjG3cDfdrSHPQirb2/cF6 6rUl9WKIplIk+pFlKmkpZJFc8Enbp/vQQtSjRCV/0Rmc456AaWHDYyCIQpTz9Ari VpTi9aGziE9KRDl+eyuPZs0lHUz6Mwn58BpFMoBbzNOVc84jna86pGfsVRENpEo= =ug+h -----END PGP SIGNATURE----- --McqAB8WelXNIERSeijLpPoBaTJonavot0--