δΊ 2014/2/13 23:14, Markus Armbruster ει:
> Wenchao Xia <xiaw...@linux.vnet.ibm.com> writes:
>
>> It will check whether the values specified are written correctly,
>> and whether all enum values are covered, when discriminator is a
>> pre-defined enum type
>>
>> Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com>
>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>> ---
>> scripts/qapi-visit.py | 17 +++++++++++++++++
>> scripts/qapi.py | 31 +++++++++++++++++++++++++++++++
>> 2 files changed, 48 insertions(+), 0 deletions(-)
>>
>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>> index 65f1a54..c0efb5f 100644
>> --- a/scripts/qapi-visit.py
>> +++ b/scripts/qapi-visit.py
>> @@ -255,6 +255,23 @@ 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' is not found in "
>> + "enum '%s'\n" %
>> + (key, enum_define["enum_name"]))
>> + sys.exit(1)
>
> Can this happen? If yes, why isn't it diagnosed in qapi.py, like all
> the other semantic errors?
>
I think the parse procedure contains two part:
1 read qapi-schema.json and parse it into exprs.
2 translate exprs into final output.
Looking at qapi.py, qapi-visit.py, qapi-types.py, it seems qapi.py is
in charge of step 1 handling literal error, and other two script are in
charge of step 2. The above error can be only detected in step 2 after
all enum defines are remembered in step 1, so I didn't add those things
into qapi.py.
I guess you want to place the check inside parse_schema() to let
test case detect it easier, one way to go is, let qapi.py do checks
for step 2:
def parse_schema(fp):
try:
schema = QAPISchema(fp)
except QAPISchemaError, e:
print >>sys.stderr, e
exit(1)
exprs = []
for expr in schema.exprs:
if expr.has_key('enum'):
add_enum(expr['enum'])
elif expr.has_key('union'):
add_union(expr)
add_enum('%sKind' % expr['union'])
elif expr.has_key('type'):
add_struct(expr)
exprs.append(expr)
+ for expr in schema.exprs:
+ if expr.has_key('union'):
+ #check code
return exprs
This way qapi.py can detect such errors. Disadvantage is that,
qapi.py is invloved for step 2 things, so some code in qapi.py
and qapi-visit.py may be dupicated, here the "if .... union...
discriminator" code may appear in both qapi.py and qapi-visit.py.
>> + for key in enum_define["enum_values"]:
>> + if not key in members:
>> + sys.stderr.write("Enum value '%s' is not covered by a
>> branch "
>> + "of union '%s'\n" %
>> + (key, name))
>> + sys.exit(1)
>> +
>
> Likewise.
>
>> ret = generate_visit_enum('%sKind' % name, members.keys())
>>
>> if base:
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index cf34768..0a3ab80 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -385,3 +385,34 @@ def guardend(name):
>>
>> ''',
>> name=guardname(name))
>> +
The funtions below are likely helper funtions, I planed to put them
into qapi_helper.py, but they are not much so kepted for easy.
>> +# This function can be used to check whether "base" is valid
>> +def find_base_fields(base):
>> + base_struct_define = find_struct(base)
>> + if not base_struct_define:
>> + return None
>> + return base_struct_define.get('data')
>> +
>> +# Return the discriminator enum define, if discriminator is specified in
>> +# @expr and it is a pre-defined enum type
>> +def discriminator_find_enum_define(expr):
>> + 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:
>> + raise StandardError("Base '%s' is not a valid type\n"
>> + % base)
>
> Why not QAPISchemaError, like for other semantic errors?
>
I think QAPISchemaError is a literal error of step 1, here
it can't be used unless we record the text/line number belong to
each expr.
>> +
>> + discriminator_type = base_fields.get(discriminator)
>> +
>> + if not discriminator_type:
>> + raise StandardError("Discriminator '%s' not found in schema\n"
>> + % discriminator)
>
> Likewise.
>
>> +
>> + return find_enum(discriminator_type)
>
> All errors should have a test in tests/qapi-schema/. I can try to add
> tests for you when I rebase your 09/10.
>
