qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH RFC v2 22/47] qapi: QAPISchema code generation h


From: Eric Blake
Subject: Re: [Qemu-devel] [PATCH RFC v2 22/47] qapi: QAPISchema code generation helper methods
Date: Mon, 27 Jul 2015 08:05:36 -0600
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0

On 07/27/2015 03:54 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>>> New methods c_name(), c_type(), c_null(), json_type(),
>>> alternate_qtype().
>>>
>>> Signed-off-by: Markus Armbruster <address@hidden>
>>> ---
>>>  scripts/qapi.py | 72
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++------
>>>  1 file changed, 65 insertions(+), 7 deletions(-)
>>>
>>
>> I just noticed:
>>
>>> @@ -779,6 +811,12 @@ class QAPISchemaEnumType(QAPISchemaType):
>>>          for v in values:
>>>              assert isinstance(v, str)
>>>          self.values = values
>>> +    def c_type(self, is_param=False):
>>> +        return c_name(self.name)
>>> +    def c_null(self):
>>> +        return c_enum_const(self.name, self.values[0])
>>
>> What does this return for an empty enum, as in { 'enum':'Empty',
>> 'data':[] }?
> 
> I suspect self.values will be [] then, and self.values[0] will bomb.
> 
> Possible fixes:
> 
> * Outlaw empty enums
> 
> * Add the implicit MAX member to self.values[] (other code may have to
>   skip it)
> 
> * Catch the special case here, and return the implicit MAX member.

I'm leaning toward the third option here.

> 
>>               Our testsuite proves we can do that, even if our normal
>> .json code doesn't use it.
> 
> tests/qapi-schema/enum-empty.json:{ 'enum': 'MyEnum', 'data': [ ] }

As I've mentioned elsewhere, most of our tests/qapi-schema/*.json merely
cover whether the parser is okay with the input, while
tests/qapi-schema/qapi-schema-test.json is the one that also tests that
the generated C code works; so sounds like that test should be enhanced
to cover some of these corner cases we have been considering in this
series (empty enum, command with no arguments and no returns, and so forth).

-- 
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]