From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41007) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdtOX-0003gX-6h for qemu-devel@nongnu.org; Tue, 05 Nov 2013 22:03:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdtOO-0001HN-57 for qemu-devel@nongnu.org; Tue, 05 Nov 2013 22:03:01 -0500 Received: from e23smtp01.au.ibm.com ([202.81.31.143]:52909) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdtON-0001HH-9B for qemu-devel@nongnu.org; Tue, 05 Nov 2013 22:02:52 -0500 Received: from /spool/local by e23smtp01.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 6 Nov 2013 13:02:42 +1000 Received: from d23relay05.au.ibm.com (d23relay05.au.ibm.com [9.190.235.152]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id A37402BB0053 for ; Wed, 6 Nov 2013 14:02:38 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id rA62iwjS6488336 for ; Wed, 6 Nov 2013 13:44:59 +1100 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id rA632aj4031775 for ; Wed, 6 Nov 2013 14:02:36 +1100 Message-ID: <5279B14E.2090301@linux.vnet.ibm.com> Date: Wed, 06 Nov 2013 11:02:38 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1383611860-9053-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1383611860-9053-4-git-send-email-xiawenc@linux.vnet.ibm.com> <5278F1E5.1070402@redhat.com> In-Reply-To: <5278F1E5.1070402@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH RFC 03/10] qapi script: check correctness of discriminator values in union List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: kwolf@redhat.com, pbonzini@redhat.com, armbru@redhat.com, lcapitulino@redhat.com 于 2013/11/5 21:25, Eric Blake 写道: > On 11/04/2013 05:37 PM, Wenchao Xia wrote: >> It will check whether the values specfied are wrotten correctly when > > s/specfied/specified/ > s/wrotten/written/ > >> discriminator is a pre-defined enum type, which help check whether the >> schema is in good form. >> >> It is allowed that, not every value in enum is used, so do not check > > s/that,/that/ > >> that case. > > Why do you allow partial coverage? That feels like an accident waiting > to happen. Does the user get a sane error message if they request an > enum value that wasn't mapped to a union branch? I think it would be > wiser to mandate that if the discriminator is an enum, then the union > must cover all values of the enum. > abort() will be called in qapi-visit.c, it is OK for output visitor, but bad for input visitor since user input may trigger it. I think change abort() to error_set() could solve the problem, then we allow map part of enum value. >> + >> +# Return the descriminator enum define, if discriminator is specified in > > s/descriminator/discriminator/ > >> +# @expr and it is a pre-defined enum type >> +def descriminator_find_enum_define(expr): > > s/descriminator/discriminator/ - and fix all callers > >> + discriminator = expr.get('discriminator') >> + base = expr.get('base') >> + >> + # Only support discriminator when base present >> + if not (discriminator and base): >> + return None >> + >> + base_fields = find_base_fields(base) >> + >> + if not base_fields: >> + sys.stderr.write("Base '%s' is not a valid type\n" >> + % base) >> + sys.exit(1) >> + >> + descriminator_type = base_fields.get(discriminator) > > s/descriminator/discriminator/ >