Re: [Qemu-devel] [PATCH v6 38/36] qapi: Check for member name conflicts with a base class

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Our type inheritance for both 'struct' and for flat 'union' merges
> key/value pairs from the base class with those from the type in
> question.  Although the C code currently boxes things so that there
> is a distinction between which member is referred to, the QMP wire
> format does not allow passing a key more than once in a single
> object.  Besides, if we ever change the generated C code to not be
> quite so boxy, we'd want to avoid duplicate member names there,
> too.
>
> Fix a testsuite entry added in an earlier patch, as well as adding
> a couple more tests to ensure we have appropriate coverage.
>
> Signed-off-by: Eric Blake <address@hidden>
[...]
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 853f9a3..281e762 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -429,6 +429,18 @@ def check_type(expr_info, source, value, allow_array = 
> False,
>                     allow_metas=['built-in', 'union', 'alternate', 'struct',
>                                  'enum'])
>
> +def check_member_clash(expr_info, base_name, data, source = ""):
> +    base = find_struct(base_name)
> +    assert base
> +    base_members = base['data']
> +    for key in data.keys():
> +        if key in base_members:
> +            raise QAPIExprError(expr_info,
> +                                "Member name '%s'%s clashes with base '%s'"
> +                                %(key, source, base_name))
> +    if base.get('base'):
> +        check_member_clash(expr_info, base['base'], data, source)
> +
>  def check_command(expr, expr_info):
>      name = expr['command']
>      allow_star = expr.has_key('gen')
> @@ -518,9 +530,14 @@ def check_union(expr, expr_info):
>          check_name(expr_info, "Member of union '%s'" % name, key)
>
>          # Each value must name a known type; futhermore, in flat unions,
> -        # branches must be a struct
> +        # branches must be a struct with no overlapping member names
>          check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
>                     value, allow_array=True, allow_metas=allow_metas)
> +        if base:
> +            branch_struct = find_struct(value)
> +            assert branch_struct
> +            check_member_clash(expr_info, expr['base'], 
> branch_struct['data'],
> +                               " of branch '%s'" %key)

Could pass base instead of expr['base'].

>
>          # If the discriminator names an enum type, then all members
>          # of 'data' must also be members of the enum type.
> @@ -597,6 +614,8 @@ def check_struct(expr, expr_info):
>                 allow_dict=True, allow_optional=True)
>      check_type(expr_info, "'base' for type '%s'" % name, expr.get('base'),
>                 allow_metas=['struct'])
> +    if expr.get('base'):
> +        check_member_clash(expr_info, expr['base'], expr['data'])
>
>  def check_exprs(schema):
>      for expr_elem in schema.exprs:
[...]

Reviewed-by: Markus Armbruster <address@hidden>