From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49078) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZL834-0003il-DE for qemu-devel@nongnu.org; Fri, 31 Jul 2015 07:00:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZL831-0004S3-1N for qemu-devel@nongnu.org; Fri, 31 Jul 2015 07:00:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57822) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZL830-0004Rv-Q4 for qemu-devel@nongnu.org; Fri, 31 Jul 2015 07:00:18 -0400 From: Markus Armbruster 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> Date: Fri, 31 Jul 2015 13:00:15 +0200 In-Reply-To: <55B93445.9060104@redhat.com> (Eric Blake's message of "Wed, 29 Jul 2015 14:15:01 -0600") Message-ID: <87mvyctww0.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain 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: Eric Blake Cc: kwolf@redhat.com, berto@igalia.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com Eric Blake writes: > On 07/29/2015 01:33 AM, Markus Armbruster wrote: >> Eric Blake writes: >> >>> On 07/01/2015 02:21 PM, Markus Armbruster wrote: >>>> The struct generated for a flat union is weird: the members of its >>>> base are at the end, except for the union tag, which is renamed to >>>> 'kind' and put at the beginning. >>> > >>> Therefore, it might be worth mentioning that avoiding the rename to >>> 'kind' is a bug fix, not just a nicer struct :) >> >> Cool! I'll work (a variation of) this test case into my series. > > 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'. Works, because schema names should not begin with '_', except for downstream extensions, which begin with '__RFQDN_'. We don't enforce that, however. I'll include the appended patch. Subject: [PATCH] qapi: Document flaws in checking of names We don't actually enforce our "other than downstream extensions [...], all names should begin with a letter" rule. Add a FIXME. We should reject names that differ only in '_' vs. '.' vs. '-', because they're liable to clash in generated C. Add a FIXME. --- scripts/qapi.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/qapi.py b/scripts/qapi.py index 4af47ef..e61db30 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -341,6 +341,8 @@ def discriminator_find_enum_define(expr): return find_enum(discriminator_type) +# FIXME should enforce "other than downstream extensions [...], all +# names should begin with a letter". valid_name = re.compile('^[a-zA-Z_][a-zA-Z0-9_.-]*$') def check_name(expr_info, source, name, allow_optional = False, enum_member = False): @@ -367,6 +369,8 @@ def check_name(expr_info, source, name, allow_optional = False, def add_name(name, info, meta, implicit = False): global all_names check_name(info, "'%s'" % meta, name) + # FIXME should reject names that differ only in '_' vs. '.' + # vs. '-', because they're liable to clash in generated C. if name in all_names: raise QAPIExprError(info, "%s '%s' is already defined" -- 2.4.3