qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType.check_clash()
Date: Mon, 9 Nov 2015 10:36:29 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0

On 11/09/2015 06:00 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Consolidate two common sequences of clash detection into a
>> new QAPISchemaObjectType.check_clash() helper method.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---

>> @@ -980,11 +980,7 @@ class QAPISchemaObjectType(QAPISchemaType):
>>          seen = OrderedDict()
>>          if self._base_name:
>>              self.base = schema.lookup_type(self._base_name)
>> -            assert isinstance(self.base, QAPISchemaObjectType)
> 
> This assertion is lost.
> 
>> -            assert not self.base.variants       # not implemented
>> -            self.base.check(schema)
>> -            for m in self.base.members:
>> -                m.check_clash(seen)
>> +            self.base.check_clash(schema, seen)

Directly lost, but indirectly still present.  The new code is calling
QAPISchemaObjectType.check_clash(), which won't exist unless self.base
is a QAPISchemaObjectType.  Folding the assert into the refactored
function makes no sense (the condition isinstance(self,
QAPISchemaObjectType) would always be true), and leaving the assert
prior to calling self.base.check_clash() adds no real protection against
programming bugs.

>> @@ -1062,12 +1064,7 @@ class QAPISchemaObjectTypeVariants(object):
>>          for v in self.variants:
>>              # Reset seen map for each variant, since qapi names from one
>>              # branch do not affect another branch
>> -            vseen = dict(seen)
>> -            assert isinstance(v.type, QAPISchemaObjectType)
> 
> This assertion is lost.
> 
>> -            assert not v.type.variants       # not implemented
>> -            v.type.check(schema)
>> -            for m in v.type.members:
>> -                m.check_clash(vseen)
>> +            v.type.check_clash(schema, dict(seen))

Same explanation.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]