Re: [Qemu-devel] [PATCH v3 06/14] qapi: require valid expressions

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> The previous patch demonstrated that the generator could get
> confused if an expression had conflicting meta-types, and
> silently ignored expressions that lacked a known meta-type.
> Fix both cases to give a sane error message.
>
> * scripts/qapi.py (check_exprs): Require a valid meta-type for
> every expression.
> * tests/qapi-schema/indented-expr.*: Use valid types.
> * tests/qapi-schema/missing-type.*: Update expected output.
> * tests/qapi-schema/double-type.*: Likewise.

Conventional git commit message followed by GNU change log.  Unusual in
QEMU.  Same elsewhere.

Explanation of the change to indented-expr.json hides in the change log
part, where I promptly missed it :)

> Signed-off-by: Eric Blake <address@hidden>
> ---
>  scripts/qapi.py                      | 12 ++++++++++++
>  tests/qapi-schema/double-type.err    |  1 +
>  tests/qapi-schema/double-type.exit   |  2 +-
>  tests/qapi-schema/double-type.out    |  3 ---
>  tests/qapi-schema/indented-expr.json |  4 ++--
>  tests/qapi-schema/indented-expr.out  |  2 +-
>  tests/qapi-schema/missing-type.err   |  1 +
>  tests/qapi-schema/missing-type.exit  |  2 +-
>  tests/qapi-schema/missing-type.out   |  3 ---
>  9 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 1082416..910e422 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -322,6 +322,18 @@ def check_exprs(schema):
>          info = expr_elem['info']
>          members = expr.get('data')
>
> +        # 'include' has already been flattened; at this point, all exprs
> +        # should have one of the remaining keys
> +        keys = (expr.has_key('enum') + expr.has_key('union') +
> +                expr.has_key('type') + expr.has_key('command') +
> +                expr.has_key('event'))
> +        if keys < 1:
> +            raise QAPIExprError(info,
> +                                "Missing expression meta-type")
> +        if keys > 1:
> +            raise QAPIExprError(info,
> +                                "Conflicting expression meta-types")
> +
>          if expr.has_key('union'):
>              check_union(expr, info)
>          if expr.has_key('event'):

Not the friendliest error messages, but they'll do.