From patchwork Mon Oct 26 22:35:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 536360 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 9CC09140E31 for ; Tue, 27 Oct 2015 09:49:05 +1100 (AEDT) Received: from localhost ([::1]:55696 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqqZb-00010L-BZ for incoming@patchwork.ozlabs.org; Mon, 26 Oct 2015 18:49:03 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55068) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqqML-0003Xi-Ua for qemu-devel@nongnu.org; Mon, 26 Oct 2015 18:35:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZqqMH-00074t-Gm for qemu-devel@nongnu.org; Mon, 26 Oct 2015 18:35:21 -0400 Received: from mx1.redhat.com ([209.132.183.28]:46163) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZqqMH-00074i-AE for qemu-devel@nongnu.org; Mon, 26 Oct 2015 18:35:17 -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 0A9BAC0AF79E; Mon, 26 Oct 2015 22:35:17 +0000 (UTC) Received: from red.redhat.com (ovpn-113-189.phx2.redhat.com [10.3.113.189]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9QMZ4Du006171; Mon, 26 Oct 2015 18:35:16 -0400 From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 26 Oct 2015 16:35:01 -0600 Message-Id: <1445898903-12082-23-git-send-email-eblake@redhat.com> In-Reply-To: <1445898903-12082-1-git-send-email-eblake@redhat.com> References: <1445898903-12082-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 v11 22/24] qapi: Finish 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 back end for a series that converts to a saner qapi union layout. Now that all clients have been converted to use 'type' and 'obj->u.value', we can drop the temporary parallel support for 'kind' and 'obj->value'. Given a simple union qapi type: { 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } } this is the overall effect, when compared to the state before this series of patches: | struct Foo { |- FooKind kind; |- union { /* union tag is @kind */ |+ FooKind type; |+ union { /* union tag is @type */ | void *data; | int64_t a; | bool b; |- }; |+ } u; | }; There are still some further changes that can be made to the testsuite now that there is no longer a collision between enum tag values and QMP names, as well as adding a reservation of 'u' to avoid a collision between QMP and our choice of union naming, but those will be later patches. 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 --- v11: enhance commit message v10: rebase to earlier changes, match commit wording 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, 5 insertions(+), 21 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 1420e00..7e35bb6 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -149,23 +149,10 @@ 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(''' - union { - %(c_type)s kind; - %(c_type)s type; - }; -''', - c_type=c_name(variants.tag_member.type.name)) + ret += gen_struct_field(variants.tag_member.name, + variants.tag_member.type, + False) - # 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 @@ -174,7 +161,7 @@ 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? - tmp += mcgen(''' + ret += mcgen(''' union { /* union tag is @%(c_name)s */ void *data; ''', @@ -183,17 +170,14 @@ struct %(c_name)s { for var in variants.variants: # Ugly special case for simple union TODO get rid of it typ = var.simple_union_type() or var.type - tmp += mcgen(''' + ret += 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; - }; }; ''')