Re: [Qemu-devel] [PATCH 02/17] qapi-types/visit.py: Inheritance for structs

[Max Reitz <mreitz@redhat.com>]

On 2013-09-20 13:54, Kevin Wolf wrote:
> This introduces a new 'base' key for struct definitions that refers to
> another struct type. On the JSON level, the fields of the base type are
> included directly into the same namespace as the fields of the defined
> type, like with unions. On the C level, a pointer to a struct of the
> base type is included.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   docs/qapi-code-gen.txt | 17 +++++++++++++++++
>   scripts/qapi-types.py  |  4 ++++
>   scripts/qapi-visit.py  | 18 ++++++++++++++++--
>   3 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0ce045c..2b872f1 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -53,6 +53,23 @@ The use of '*' as a prefix to the name means the member is optional.  Optional
>   members should always be added to the end of the dictionary to preserve
>   backwards compatibility.
>   
> +
> +A complex type definition can specify another complex type as its base.
> +In this case, the fields of the base type are included as top-level fields
> +of the new complex type's dictionary in the QMP wire format. An example
> +definition is:
> +
> + { 'type': 'BlockdevOptionsGenericFormat', 'data': { 'file': 'str' } }
> + { 'type': 'BlockdevOptionsGenericCOWFormat',
> +   'base': 'BlockdevOptionsGenericFormat',
> +   'data': { '*backing': 'str' } }
> +
> +An example BlockdevOptionsGenericCOWFormat object on the wire could use
> +both fields like this:
> +
> + { "file": "/some/place/my-image"
> +   "backing": "/some/place/my-backing-file" } }
> +
>   === Enumeration types ===
>   
>   An enumeration type is a dictionary containing a single key whose value is a
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 566fe5e..4a1652b 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -86,6 +86,7 @@ def generate_struct(expr):
>       structname = expr.get('type', "")
>       fieldname = expr.get('field', "")
>       members = expr['data']
> +    base = expr.get('base')
>   
>       ret = mcgen('''
>   struct %(name)s
> @@ -93,6 +94,9 @@ struct %(name)s
>   ''',
>             name=structname)
>   
> +    if base:
> +        ret += generate_struct_fields({'base': base})
> +
>       ret += generate_struct_fields(members)
>   
>       if len(fieldname):
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 1e44004..90cedd7 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -17,7 +17,7 @@ import os
>   import getopt
>   import errno
>   
> -def generate_visit_struct_fields(name, field_prefix, fn_prefix, members):
> +def generate_visit_struct_fields(name, field_prefix, fn_prefix, members, base = None):
>       substructs = []
>       ret = ''
>       full_name = name if not fn_prefix else "%s_%s" % (name, fn_prefix)
> @@ -42,6 +42,19 @@ static void visit_type_%(full_name)s_fields(Visitor *m, %(name)s ** obj, Error *
>           name=name, full_name=full_name)
>       push_indent()
>   
> +    if base:
> +        ret += mcgen('''
> +visit_start_implicit_struct(m, (void**) &(*obj)->%(c_name)s, sizeof(%(type)s), &err);
Why do you just dereference obj here (implying it will never be NULL)...

> +if (!err) {
> +    visit_type_%(type)s_fields(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, &err);
...but then you're checking whether it is NULL before dereferencing?

Max

> +    error_propagate(errp, err);
> +    err = NULL;
> +    visit_end_implicit_struct(m, &err);
> +}
> +''',
> +                     c_prefix=c_var(field_prefix),
> +                     type=type_name(base), c_name=c_var('base'))
> +
>       for argname, argentry, optional, structured in parse_args(members):
>           if optional:
>               ret += mcgen('''
> @@ -124,8 +137,9 @@ def generate_visit_struct(expr):
>   
>       name = expr['type']
>       members = expr['data']
> +    base = expr.get('base')
>   
> -    ret = generate_visit_struct_fields(name, "", "", members)
> +    ret = generate_visit_struct_fields(name, "", "", members, base)
>   
>       ret += mcgen('''
>