Commit cf393590 authored by Eric Blake's avatar Eric Blake Committed by Markus Armbruster
Browse files

qapi: Better error messages for bad enums 



The previous commit demonstrated that the generator had several
flaws with less-than-perfect enums:
- an enum that listed the same string twice (or two variant
strings that map to the same C enumerator) ended up generating
an invalid C enum
- because the generator adds a _MAX terminator to each enum,
the use of an enum member 'max' can also cause this clash
- if an enum omits 'data', the generator left a python stack
trace rather than a graceful message
- an enum that used a non-array 'data' was silently accepted by
the parser
- an enum that used non-string members in the 'data' member
was silently accepted by the parser

Add check_enum to cover these situations, and update testcases
to match.  While valid .json files won't trigger any of these
cases, we might as well be nicer to developers that make a typo
while trying to add new QAPI code.

Signed-off-by: default avatarEric Blake <eblake@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent ad11dbb9

scripts/qapi.py

+29 −5
Original line number Diff line number Diff line
@@ -311,13 +311,37 @@ def check_union(expr, expr_info): 
        # Todo: add checking for values. Key is checked as above, value can be

        # also checked here, but we need more functions to handle array case.



def check_enum(expr, expr_info):

    name = expr['enum']

    members = expr.get('data')

    values = { 'MAX': '(automatic)' }



    if not isinstance(members, list):

        raise QAPIExprError(expr_info,

                            "Enum '%s' requires an array for 'data'" % name)

    for member in members:

        if not isinstance(member, str):

            raise QAPIExprError(expr_info,

                                "Enum '%s' member '%s' is not a string"

                                % (name, member))

        key = _generate_enum_string(member)

        if key in values:

            raise QAPIExprError(expr_info,

                                "Enum '%s' member '%s' clashes with '%s'"

                                % (name, member, values[key]))

        values[key] = member



def check_exprs(schema):

    for expr_elem in schema.exprs:

        expr = expr_elem['expr']

        if expr.has_key('union'):

            check_union(expr, expr_elem['info'])

        if expr.has_key('event'):

            check_event(expr, expr_elem['info'])

        info = expr_elem['info']



        if expr.has_key('enum'):

            check_enum(expr, info)

        elif expr.has_key('union'):

            check_union(expr, info)

        elif expr.has_key('event'):

            check_event(expr, info)



def parse_schema(input_file):

    try:
@@ -331,7 +355,7 @@ def parse_schema(input_file): 
    for expr_elem in schema.exprs:

        expr = expr_elem['expr']

        if expr.has_key('enum'):

            add_enum(expr['enum'], expr['data'])

            add_enum(expr['enum'], expr.get('data'))

        elif expr.has_key('union'):

            add_union(expr)

        elif expr.has_key('type'):

tests/qapi-schema/enum-clash-member.err

+1 −0
Original line number Diff line number Diff line 
tests/qapi-schema/enum-clash-member.json:2: Enum 'MyEnum' member 'ONE' clashes with 'one'

tests/qapi-schema/enum-clash-member.exit

+1 −1
Original line number Diff line number Diff line 
0
 
1

tests/qapi-schema/enum-clash-member.json

+1 −1
Original line number Diff line number Diff line 
# FIXME: we should reject enums where members will clash when mapped to C enum

# we reject enums where members will clash when mapped to C enum

{ 'enum': 'MyEnum', 'data': [ 'one', 'ONE' ] }

tests/qapi-schema/enum-clash-member.out

+0 −3
Original line number Diff line number Diff line 
[OrderedDict([('enum', 'MyEnum'), ('data', ['one', 'ONE'])])]

[{'enum_name': 'MyEnum', 'enum_values': ['one', 'ONE']}]

[]