From patchwork Thu Feb 18 06:48:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 8346621 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 6BC209F399 for ; Thu, 18 Feb 2016 06:51:05 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A2B4D20395 for ; Thu, 18 Feb 2016 06:51:04 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id ADA4020386 for ; Thu, 18 Feb 2016 06:51:03 +0000 (UTC) Received: from localhost ([::1]:37661 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWIQZ-0000zu-4Q for patchwork-qemu-devel@patchwork.kernel.org; Thu, 18 Feb 2016 01:51:03 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40201) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWIOE-0005Mh-PN for qemu-devel@nongnu.org; Thu, 18 Feb 2016 01:48:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWIOD-000207-9V for qemu-devel@nongnu.org; Thu, 18 Feb 2016 01:48:38 -0500 Received: from mx1.redhat.com ([209.132.183.28]:58936) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWIOD-0001zw-1t for qemu-devel@nongnu.org; Thu, 18 Feb 2016 01:48:37 -0500 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id A10298F4FD; Thu, 18 Feb 2016 06:48:36 +0000 (UTC) Received: from red.redhat.com (ovpn-113-75.phx2.redhat.com [10.3.113.75]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1I6mXAb023855; Thu, 18 Feb 2016 01:48:36 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Wed, 17 Feb 2016 23:48:20 -0700 Message-Id: <1455778109-6278-7-git-send-email-eblake@redhat.com> In-Reply-To: <1455778109-6278-1-git-send-email-eblake@redhat.com> References: <1455778109-6278-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 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 06/15] qapi: Visit variants in visit_type_FOO_fields() 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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP We initially created the static visit_type_FOO_fields() helper function for reuse of code - we have cases where the initial setup for a visit has different allocation (depending on whether the fields represent a stand-alone type or are embedded as part of a larger type), but where the actual field visits are identical once a pointer is available. Up until the previous patch, visit_type_FOO_fields() was only used for structs (no variants), so it was covering every field for each type where it was emitted. Meanwhile, the code for visiting unions looks like: static visit_type_U_fields() { visit base; visit local_members; } visit_type_U() { visit_start_struct(); visit_type_U_fields(); visit variants; visit_end_struct(); } which splits the fields of the union visit across two functions. Move the code to visit variants to live inside visit_type_U_fields(), while making it conditional on having variants so that all other instances of the helper function remain unchanged. This is also a step closer towards unifying struct and union visits, and towards allowing one union type to be the branch of another flat union. The resulting diff to the generated code is a bit hard to read, but it can be verified that it touches only union types, and that the end result is the following general structure: static visit_type_U_fields() { visit base; visit local_members; visit variants; } visit_type_U() { visit_start_struct(); visit_type_U_fields(); visit_end_struct(); } Signed-off-by: Eric Blake --- v11: new patch --- scripts/qapi-visit.py | 104 +++++++++++++++++++++++++------------------------- 1 file changed, 53 insertions(+), 51 deletions(-) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 1051710..6cea7d6 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -71,11 +71,16 @@ static void visit_type_implicit_%(c_type)s(Visitor *v, %(c_type)s **obj, Error * return ret -def gen_visit_struct_fields(name, base, members): +def gen_visit_struct_fields(name, base, members, variants=None): ret = '' if base: ret += gen_visit_fields_decl(base) + if variants: + for var in variants.variants: + # Ugly special case for simple union TODO get rid of it + if not var.simple_union_type(): + ret += gen_visit_implicit_struct(var.type) struct_fields_seen.add(name) ret += mcgen(''' @@ -96,8 +101,49 @@ static void visit_type_%(c_name)s_fields(Visitor *v, %(c_name)s **obj, Error **e ret += gen_visit_fields(members, prefix='(*obj)->') - # 'goto out' produced for base, and by gen_visit_fields() for each member - if base or members: + if variants: + ret += mcgen(''' + if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) { + goto out; + } + switch ((*obj)->%(c_name)s) { +''', + c_name=c_name(variants.tag_member.name)) + + for var in variants.variants: + # TODO ugly special case for simple union + simple_union_type = var.simple_union_type() + ret += mcgen(''' + case %(case)s: +''', + case=c_enum_const(variants.tag_member.type.name, + var.name, + variants.tag_member.type.prefix)) + if simple_union_type: + ret += mcgen(''' + visit_type_%(c_type)s(v, "data", &(*obj)->u.%(c_name)s, &err); +''', + c_type=simple_union_type.c_name(), + c_name=c_name(var.name)) + else: + ret += mcgen(''' + visit_type_implicit_%(c_type)s(v, &(*obj)->u.%(c_name)s, &err); +''', + c_type=var.type.c_name(), + c_name=c_name(var.name)) + ret += mcgen(''' + break; +''') + + ret += mcgen(''' + default: + abort(); + } +''') + + # 'goto out' produced for base, by gen_visit_fields() for each member, + # and if variants were present + if base or members or variants: ret += mcgen(''' out: @@ -239,12 +285,7 @@ out: def gen_visit_union(name, base, members, variants): - ret = gen_visit_struct_fields(name, base, members) - - for var in variants.variants: - # Ugly special case for simple union TODO get rid of it - if not var.simple_union_type(): - ret += gen_visit_implicit_struct(var.type) + ret = gen_visit_struct_fields(name, base, members, variants) ret += mcgen(''' @@ -260,54 +301,15 @@ void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error goto out_obj; } visit_type_%(c_name)s_fields(v, obj, &err); -''', - c_name=c_name(name)) - ret += gen_err_check(label='out_obj') - ret += mcgen(''' - if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) { - goto out_obj; - } - switch ((*obj)->%(c_name)s) { -''', - c_name=c_name(variants.tag_member.name)) - - for var in variants.variants: - # TODO ugly special case for simple union - simple_union_type = var.simple_union_type() - ret += mcgen(''' - case %(case)s: -''', - case=c_enum_const(variants.tag_member.type.name, - var.name, - variants.tag_member.type.prefix)) - if simple_union_type: - ret += mcgen(''' - visit_type_%(c_type)s(v, "data", &(*obj)->u.%(c_name)s, &err); -''', - c_type=simple_union_type.c_name(), - c_name=c_name(var.name)) - else: - ret += mcgen(''' - visit_type_implicit_%(c_type)s(v, &(*obj)->u.%(c_name)s, &err); -''', - c_type=var.type.c_name(), - c_name=c_name(var.name)) - ret += mcgen(''' - break; -''') - - ret += mcgen(''' - default: - abort(); - } -out_obj: error_propagate(errp, err); err = NULL; +out_obj: visit_end_struct(v, &err); out: error_propagate(errp, err); } -''') +''', + c_name=c_name(name)) return ret