Re: [Qemu-devel] [PATCH 3/8] qapi script: check correctness of discriminator values in union

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/11/13 2:12, Eric Blake 写道:
> On 11/06/2013 12:33 PM, Wenchao Xia wrote:
>> It will check whether the values specified are written correctly when
>> 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 does not check
>> that case.
>
> Again, I think you should require that every value in the enum is used.
>
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   scripts/qapi-visit.py |   11 +++++++++++
>>   scripts/qapi.py       |   33 +++++++++++++++++++++++++++++++++
>>   2 files changed, 44 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>> index b3d3af8..612dc4d 100644
>> --- a/scripts/qapi-visit.py
>> +++ b/scripts/qapi-visit.py
>> @@ -251,6 +251,17 @@ def generate_visit_union(expr):
>>           assert not base
>>           return generate_visit_anon_union(name, members)
>>
>> +    # If discriminator is specified and it is a pre-defined enum in schema,
>> +    # check its correctness
>> +    enum_define = discriminator_find_enum_define(expr)
>> +    if enum_define:
>> +        for key in members:
>> +            if not key in enum_define["enum_values"]:
>> +                sys.stderr.write("Discriminator value '%s' not found in "
>> +                                 "enum '%s'\n" %
>> +                                 (key, enum_define["enum_name"]))
>> +                sys.exit(1)
>
> This checks for union branches not covered by the enum, but does not
> check for duplicate union branches, nor does it check for enum values
> not covered by a union branch.
>
   I agree that, allowing enum values not covered by union branch,
require more carefully error handling for caller, such as visitors,
will add the check for safty reason.