On 07/27/2015 03:54 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com>
>>> ---
>>> 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
signature.asc
Description: OpenPGP digital signature
