Re: [Qemu-devel] [PATCH v2] qapi: add struct strList and visit_type_strList()

[mdroth <mdroth@linux.vnet.ibm.com>]

On Thu, Apr 25, 2013 at 12:08:05AM +0800, Amos Kong wrote:
> Currently we can only use ['String'] to add string to a list,
> it contains some additional JSON structure.
>     "multicast": [
>         {
>             "str": "01:80:c2:00:00:21"
>         },
>         {
>             "str": "00:00:00:00:00:00"
>         }
>     ]
> 
> This patch adds struct strList and visit function, we can use ['str'] in
> schema file.
> 
>     "multicast": [
>         "01:00:5e:00:00:01",
>         "33:33:ff:12:34:57"
>     ]
> 
> V2: remove ugly #ifndef, add forward declaration in qapi-types.h,
>     and define the struct in include/qapi/visitor.h (Paolo)
> 
> Signed-off-by: Amos Kong <akong@redhat.com>

Sorry for the delay in getting back to you, I started to respond last
week and ended up hacking something up that take a different approach.

Namely: we don't hardcode visit_type_strList(), but instead teach the
qapi code generators about native types we currently allow in schemas
('str', 'int', 'number', 'bool') and have them handle code generation
for these like they would for any user-defined type.

This approach works too, but I think this solution is more complete.
I'm still working out some bits with the unit tests but I've pushed my
WIP here for reference:

https://github.com/mdroth/qemu/commits/qapi-native-lists

Please give that a shot.

> ---
>  include/qapi/visitor.h |    7 +++++++
>  qapi/qapi-visit-core.c |   23 +++++++++++++++++++++++
>  scripts/qapi-types.py  |    1 +
>  scripts/qapi.py        |    4 ++--
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index 1fef18c..743ff92 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -22,6 +22,11 @@ typedef struct GenericList
>      struct GenericList *next;
>  } GenericList;
> 
> +typedef struct strList {
> +    char *value;
> +    struct strList *next;
> +} strList;
> +
>  typedef struct Visitor Visitor;
> 
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -50,6 +55,8 @@ void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp);
>  void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
> +void visit_type_strList(Visitor *m, strList ** obj, const char *name,
> +                        Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> 
>  #endif
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 401ee6e..dc54cc8 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -257,6 +257,29 @@ void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp)
>      }
>  }
> 
> +void visit_type_strList(Visitor *m, strList ** obj, const char *name,
> +                        Error **errp)
> +{
> +    GenericList *i, **prev = (GenericList **)obj;
> +    Error *err = NULL;
> +
> +    if (!error_is_set(errp)) {
> +        visit_start_list(m, name, &err);
> +        if (!err) {
> +            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
> +                strList *native_i = (strList *)i;
> +                visit_type_str(m, &native_i->value, NULL, &err);
> +            }
> +            error_propagate(errp, err);
> +            err = NULL;
> +
> +            /* Always call end_list if start_list succeeded.  */
> +            visit_end_list(m, &err);
> +        }
> +        error_propagate(errp, err);
> +    }
> +}
> +
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp)
>  {
>      if (!error_is_set(errp)) {
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9e19920..e449a14 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -276,6 +276,7 @@ fdecl.write(mcgen('''
>  #include <stdbool.h>
>  #include <stdint.h>
> 
> +struct strList;
>  ''',
>                    guard=guardname(h_file)))
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index afc5f32..14f9f4d 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -166,11 +166,11 @@ def c_fun(name, protect=True):
>      return c_var(name, protect).replace('.', '_')
> 
>  def c_list_type(name):
> -    return '%sList' % name
> +    return 'struct %sList' % name
> 
>  def type_name(name):
>      if type(name) == list:
> -        return c_list_type(name[0])
> +        return '%sList' % name[0]
>      return name
> 
>  enum_types = []
> -- 
> 1.7.1
>