From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37829) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKobv-00085M-4j for qemu-devel@nongnu.org; Thu, 30 Jul 2015 10:15:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZKobq-0006LK-1V for qemu-devel@nongnu.org; Thu, 30 Jul 2015 10:15:03 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60522) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZKobp-0006L4-QH for qemu-devel@nongnu.org; Thu, 30 Jul 2015 10:14:57 -0400 References: <1435782155-31412-1-git-send-email-armbru@redhat.com> <1435782155-31412-8-git-send-email-armbru@redhat.com> <55B7E15D.50902@redhat.com> <87lhdzju4i.fsf@blackfin.pond.sub.org> <55B93445.9060104@redhat.com> <87io923ysb.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <55BA3157.2070806@redhat.com> Date: Thu, 30 Jul 2015 08:14:47 -0600 MIME-Version: 1.0 In-Reply-To: <87io923ysb.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="hn8Ji7JAUhiEK3DD4FcdKRs0FiutQxMSv" Subject: Re: [Qemu-devel] [PATCH RFC v2 07/47] qapi: Generate a nicer struct for flat unions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: kwolf@redhat.com, berto@igalia.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --hn8Ji7JAUhiEK3DD4FcdKRs0FiutQxMSv Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/30/2015 01:11 AM, Markus Armbruster wrote: >> Another name collision bug: our code generates flat unions as: >> >> struct BlockdevOptions { >> BlockdevDriver driver; >> ... >> /* End fields inherited from BlockdevOptionsBase. */ >> /* union tag is BlockdevDriver driver */ >> union { >> void *data; >> BlockdevOptionsArchipelago *archipelago; >> ... >> >> which means that if we name any of the branches 'data' (that is, if >> 'data' is a member of the enum discriminator), things fail to compile.= >> We could probably fix that by naming our dummy branch '_data'. >=20 > I wonder whether member data is actually used. I'll find out. The dealloc visitor uses 'data' being non-null as a flag on whether to deallocate the union even if the tag was invalid for some reason; or more importantly, if parsing consumed the tag but then detected an error while parsing the union, leaving the union branch partially allocated. To avoid a leak, we have to deallocate the branch. But if the tag was invalid, then why did we ever allocate the union in the first place, and how do we prove we are calling the correct free-ing function? And if the tag is valid, why can't we just guarantee that the union is 0-initialized and that deleting the branch will work through the correct branch type instead of worrying about 'data'? We still need a dummy member if it is valid to do { 'union':'Foo', 'data':{} } since C doesn't like empty unions, but an empty union seems like something we may want to reject, at which point you are probably right that deleting the data member altogether should work and still let us recover from bad partial parses without a leak. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --hn8Ji7JAUhiEK3DD4FcdKRs0FiutQxMSv 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/ iQEcBAEBCAAGBQJVujFXAAoJEKeha0olJ0NqOL0H/Rf5o8GBMLfRYKyS3dDbNTAP lJFOJkJBl4F0y/YfJGTT3yGnzRCcpg+zit1rQnYsP2oXl71VNH5E6bULpBcVgTwR AuK5wR6bFhJxkUMepKou1FpIY5v3PWlQ72UWQuyc4/Yn9Yo+3M/FTpICR9NQfeKI /Q7aoe22zOOH7JOuEusmbuUMXU+UzM0Adh8PounDcKbF71IPoiWelzh5/SAZ+B/f /S0/QpeOvvQefDGg9tRUJBNNoE32L69c9kNCHDIt5Xad8xL7UoFhLwlsWKwWTcgL BcHwU/FP2QaD7Rs1Xvv2q8g/i/wdKAcZEUm5uUWie6/YQqZVsRGYhOSrzc1xgIk= =MxwM -----END PGP SIGNATURE----- --hn8Ji7JAUhiEK3DD4FcdKRs0FiutQxMSv--