Re: [Qemu-devel] [PATCH v6 10/27] qapi: factor out checking for keys

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Introduce a new helper function to check if the given keys are known,
> and if mandatory keys are present. The function will be reused in
> other places in the following code changes.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/common.py | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index a353670079..8313c478c4 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -873,6 +873,17 @@ def check_struct(expr, info):
>                 allow_metas=['struct'])
>  
>  
> +def check_known_keys(info, source, keys, required, optional):
> +    for key in keys:
> +        if key not in required and key not in optional:
> +            raise QAPISemError(info, "Unknown key '%s' in %s" % (key, source))
> +
> +    for key in required:
> +        if key not in keys:
> +            raise QAPISemError(info, "Key '%s' is missing from %s"
> +                               % (key, source))
> +
> +
>  def check_keys(expr_elem, meta, required, optional=[]):
>      expr = expr_elem['expr']
>      info = expr_elem['info']
> @@ -880,10 +891,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
>      if not isinstance(name, str):
>          raise QAPISemError(info, "'%s' key must have a string value" % meta)
>      required = required + [meta]
> +    source = "%s '%s'" % (meta, name)
> +    check_known_keys(info, source, list(expr.keys()), required, optional)

Sure you need list() here?

>      for (key, value) in expr.items():
> -        if key not in required and key not in optional:
> -            raise QAPISemError(info, "Unknown key '%s' in %s '%s'"
> -                               % (key, meta, name))
>          if key in ['gen', 'success-response'] and value is not False:
>              raise QAPISemError(info,
>                                 "'%s' of %s '%s' should only use false value"
> @@ -895,10 +905,6 @@ def check_keys(expr_elem, meta, required, optional=[]):
>                                 % (key, meta, name))
>          if key == 'if':
>              check_if(expr, info)
> -    for key in required:
> -        if key not in expr:
> -            raise QAPISemError(info, "Key '%s' is missing from %s '%s'"
> -                               % (key, meta, name))
>  
>  
>  def check_exprs(exprs):