From patchwork Fri Oct 23 05:09:45 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 534778 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 59191141333 for ; Fri, 23 Oct 2015 16:12:53 +1100 (AEDT) Received: from localhost ([::1]:36039 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpUep-0003J1-1R for incoming@patchwork.ozlabs.org; Fri, 23 Oct 2015 01:12:51 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpUcC-0006xx-LE for qemu-devel@nongnu.org; Fri, 23 Oct 2015 01:10:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpUcB-0003Hl-7F for qemu-devel@nongnu.org; Fri, 23 Oct 2015 01:10:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58660) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpUcB-0003Hd-0t for qemu-devel@nongnu.org; Fri, 23 Oct 2015 01:10:07 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 9886AAB8; Fri, 23 Oct 2015 05:10:06 +0000 (UTC) Received: from red.redhat.com (ovpn-113-176.phx2.redhat.com [10.3.113.176]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9N59xED026589; Fri, 23 Oct 2015 01:10:06 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Thu, 22 Oct 2015 23:09:45 -0600 Message-Id: <1445576998-2921-13-git-send-email-eblake@redhat.com> In-Reply-To: <1445576998-2921-1-git-send-email-eblake@redhat.com> References: <1445576998-2921-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: armbru@redhat.com, Michael Roth Subject: [Qemu-devel] [PATCH v10 12/25] qapi: Start converting to new qapi union layout X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org We have two issues with our qapi union layout: 1) Even though the QMP wire format spells the tag 'type', the C code spells it 'kind', requiring some hacks in the generator. 2) The C struct uses an anonymous union, which places all tag values in the same namespace as all non-variant members. This leads to spurious collisions if a tag value matches a QMP name. This patch is the front end for a series that converts to a saner qapi union layout. By the end of the series, we will no longer have the type/kind mismatch, and all tag values will be under a named union, which requires clients to access 'obj->u.value' instead of 'obj->value'. But since the conversion touches a number of files, it is easiest if we temporarily support BOTH layouts simultaneously. Given a simple union qapi type: { 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } } make the following changes in generated qapi-types.h: | struct Foo { |- FooKind kind; |- union { /* union tag is @kind */ |+ union { |+ FooKind kind; |+ FooKind type; |+ }; |+ union { /* union tag is @type */ | void *data; | int64_t a; | bool b; |+ union { /* union tag is @type */ |+ void *data; |+ int64_t a; |+ bool b; |+ } u; | }; | }; Flat unions do not need the anonymous union for the tag member, as we already fixed that to use the member name instead of 'kind' back in commit 0f61af3e. Later, when the conversions are complete, we will remove the duplication hacks. Note, however, that we do not rename the generated enum, which is still 'FooKind'. A further patch could generate implicit enums as 'FooType', but while the generator already reserved the '*Kind' namespace (commit 4dc2e69), there are already QMP constructs with '*Type' naming, which means changing our reservation namespace would have lots of churn to C code to deal with a forced name change. Signed-off-by: Eric Blake --- v10: rebase to earlier changes, improve commit message, defer qapi-visit and testsuite 'u' changes to later v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC: http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html --- scripts/qapi-types.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 9c51d8f..579532d 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -148,11 +148,23 @@ struct %(c_name)s { if base: ret += gen_struct_fields([], base) else: + # TODO As a hack, we emit both 'kind' and 'type'. Ultimately, we + # want to use only 'type', but the conversion is large enough to + # require staging over several commits. ret += mcgen(''' - %(c_type)s kind; + union { + %(c_type)s kind; + %(c_type)s type; + }; ''', c_type=c_name(variants.tag_member.type.name)) + # TODO As a hack, we emit the union twice, once as an anonymous union + # and once as a named union. Ultimately, we want to use only the + # named union version (as it avoids conflicts between tag values as + # branch names competing with non-variant QMP names), but the conversion + # is large enough to require staging over several commits. + tmp = '' # FIXME: What purpose does data serve, besides preventing a union that # has a branch named 'data'? We use it in qapi-visit.py to decide # whether to bypass the switch statement if visiting the discriminator @@ -161,25 +173,25 @@ struct %(c_name)s { # should not be any data leaks even without a data pointer. Or, if # 'data' is merely added to guarantee we don't have an empty union, # shouldn't we enforce that at .json parse time? - ret += mcgen(''' + tmp += mcgen(''' union { /* union tag is @%(c_name)s */ void *data; ''', - # TODO ugly special case for simple union - # Use same tag name in C as on the wire to get rid of - # it, then: c_name=c_name(variants.tag_member.name) - c_name=c_name(variants.tag_name or 'kind')) + c_name=c_name(variants.tag_member.name)) for var in variants.variants: # Ugly special case for simple union TODO get rid of it typ = var.simple_union_type() or var.type - ret += mcgen(''' + tmp += mcgen(''' %(c_type)s %(c_name)s; ''', c_type=typ.c_type(), c_name=c_name(var.name)) + ret += tmp + ret += ' ' + '\n '.join(tmp.split('\n')) ret += mcgen(''' + } u; }; }; ''')