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

[Wenchao Xia]

于 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 <address@hidden>
> > ---
> >   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.