Patchwork [v3,05/22] qapi: qapi_visit.py, support arrays and complex qapi definitions

login
register
mail settings
Submitter Michael Roth
Date Oct. 4, 2012, 5:33 p.m.
Message ID <1349372021-31212-6-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/189305/
State New
Headers show

Comments

Michael Roth - Oct. 4, 2012, 5:33 p.m.
Add support for arrays in the code generators.

Complex field descriptions can now be used to provide additional
information to the visitor generators, such as the max size of an array,
or the field within a struct to use to determine how many elements are
present in the array to avoid serializing uninitialized elements.

Add handling for these in the code generators as well.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi.py          |    8 ++++--
 scripts/qapi_commands.py |    8 +++---
 scripts/qapi_types.py    |    2 +-
 scripts/qapi_visit.py    |   64 +++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 68 insertions(+), 14 deletions(-)
Paolo Bonzini - Oct. 5, 2012, 8:11 a.m.
Il 04/10/2012 19:33, Michael Roth ha scritto:
> Add support for arrays in the code generators.
> 
> Complex field descriptions can now be used to provide additional
> information to the visitor generators, such as the max size of an array,
> or the field within a struct to use to determine how many elements are
> present in the array to avoid serializing uninitialized elements.
> 
> Add handling for these in the code generators as well.
> 
> Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  scripts/qapi.py          |    8 ++++--
>  scripts/qapi_commands.py |    8 +++---
>  scripts/qapi_types.py    |    2 +-
>  scripts/qapi_visit.py    |   64 +++++++++++++++++++++++++++++++++++++++++-----
>  4 files changed, 68 insertions(+), 14 deletions(-)
> 
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 122b4cb..a347203 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -110,12 +110,16 @@ def parse_args(typeinfo):
>          argentry = typeinfo[member]
>          optional = False
>          structured = False
> +        annotated = False
>          if member.startswith('*'):
>              argname = member[1:]
>              optional = True
>          if isinstance(argentry, OrderedDict):
> -            structured = True
> -        yield (argname, argentry, optional, structured)
> +            if argentry.has_key('<annotated>'):
> +                annotated = True
> +            else:
> +                structured = True
> +        yield (argname, argentry, optional, structured, annotated)
>  
>  def de_camel_case(name):
>      new_name = ''
> diff --git a/scripts/qapi_commands.py b/scripts/qapi_commands.py
> index 3c4678d..a2e0c23 100644
> --- a/scripts/qapi_commands.py
> +++ b/scripts/qapi_commands.py
> @@ -32,7 +32,7 @@ void %(visitor)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
>  
>  def generate_command_decl(name, args, ret_type):
>      arglist=""
> -    for argname, argtype, optional, structured in parse_args(args):
> +    for argname, argtype, optional, structured, annotated in parse_args(args):
>          argtype = c_type(argtype)
>          if argtype == "char *":
>              argtype = "const char *"
> @@ -50,7 +50,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
>      retval=""
>      if ret_type:
>          retval = "retval = "
> -    for argname, argtype, optional, structured in parse_args(args):
> +    for argname, argtype, optional, structured, annotated in parse_args(args):
>          if optional:
>              arglist += "has_%s, " % c_var(argname)
>          arglist += "%s, " % (c_var(argname))
> @@ -106,7 +106,7 @@ Visitor *v;
>  def gen_visitor_input_vars_decl(args):
>      ret = ""
>      push_indent()
> -    for argname, argtype, optional, structured in parse_args(args):
> +    for argname, argtype, optional, structured, annotated in parse_args(args):
>          if optional:
>              ret += mcgen('''
>  bool has_%(argname)s = false;
> @@ -145,7 +145,7 @@ v = qmp_input_get_visitor(mi);
>  ''',
>                       obj=obj)
>  
> -    for argname, argtype, optional, structured in parse_args(args):
> +    for argname, argtype, optional, structured, annotated in parse_args(args):
>          if optional:
>              ret += mcgen('''
>  visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
> diff --git a/scripts/qapi_types.py b/scripts/qapi_types.py
> index 49ef569..6518768 100644
> --- a/scripts/qapi_types.py
> +++ b/scripts/qapi_types.py
> @@ -45,7 +45,7 @@ struct %(name)s
>  ''',
>            name=structname)
>  
> -    for argname, argentry, optional, structured in parse_args(members):
> +    for argname, argentry, optional, structured, annotated in parse_args(members):
>          if optional:
>              ret += mcgen('''
>      bool has_%(c_name)s;
> diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
> index 974e458..b3f34a2 100644
> --- a/scripts/qapi_visit.py
> +++ b/scripts/qapi_visit.py
> @@ -16,6 +16,52 @@ import sys
>  import os
>  import getopt
>  import errno
> +import types
> +
> +def generate_visit_carray_body(name, info):
> +    if info['array_size'][0].isdigit():
> +        array_size = info['array_size']
> +    elif info['array_size'][0] == '(' and info['array_size'][-1] == ')':
> +        array_size = info['array_size']
> +    else:
> +        array_size = "(*obj)->%s" % info['array_size']
> +
> +    if info.has_key('array_capacity'):
> +        array_capacity = info['array_capacity']
> +    else:
> +        array_capacity = array_size
> +
> +    if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:

Please add a comment that this is for array of objects vs. array of pointers.

> +        ret = mcgen('''
> +visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
> +int %(name)s_i;
> +for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
> +    %(type)s %(name)s_ptr = &(*obj)->%(name)s[%(name)s_i];
> +    visit_next_carray(m, errp);
> +    visit_type_%(type_short)s(m, &%(name)s_ptr, NULL, errp);
> +}
> +visit_end_carray(m, errp);

This may leave unbalanced items on the stack.  See the idiom that is used for
lists.

    if (!error_is_set(errp)) {
        visit_start_list(m, name, &err);
        if (!err) {
            for (; (i = visit_next_list(m, prev, &err)) != NULL; prev = &i) {
                %(name)sList *native_i = (%(name)sList *)i;
                visit_type_%(name)s(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);
    }


> +''',
> +                    name=name, type=c_type(info['type'][0]),
> +                    type_short=info['type'][0],
> +                    array_size=array_size,
> +                    array_capacity=array_capacity)
> +    else:
> +        ret = mcgen('''
> +visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
> +int %(name)s_i;
> +for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
> +    %(type)s *%(name)s_ptr = (%(type)s *)&(*obj)->%(name)s[%(name)s_i];

Is the cast needed?

> +    visit_next_carray(m, errp);
> +    visit_type_%(type_short)s(m, %(name)s_ptr, NULL, errp);
> +}
> +visit_end_carray(m, errp);
> +''',
> +                    name=name, type=c_type(info['type'][0]),
> +                    type_short=info['type'][0],
> +                    array_size=array_size,
> +                    array_capacity=array_capacity)
> +    return ret
>  
>  def generate_visit_struct_body(field_prefix, name, members):
>      ret = mcgen('''
> @@ -45,10 +91,10 @@ if (!err) {
>  
>      push_indent()
>      push_indent()
> -    for argname, argentry, optional, structured in parse_args(members):
> +    for argname, argentry, optional, structured, annotated in parse_args(members):
>          if optional:
>              ret += mcgen('''
> -visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", &err);
> +visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", errp);
>  if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
>  ''',
>                           c_prefix=c_var(field_prefix), prefix=field_prefix,
> @@ -58,12 +104,16 @@ if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
>          if structured:
>              ret += generate_visit_struct_body(field_prefix + argname, argname, argentry)
>          else:
> -            ret += mcgen('''
> -visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", &err);
> +            if annotated:
> +                if isinstance(argentry['type'], types.ListType):
> +                    ret += generate_visit_carray_body(argname, argentry)

Please add documentation to docs/qapi-code-gen.txt for this and the other patches.

> +            else:
> +                ret += mcgen('''
> +visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", errp);
>  ''',
> -                         c_prefix=c_var(field_prefix), prefix=field_prefix,
> -                         type=type_name(argentry), c_name=c_var(argname),
> -                         name=argname)
> +                             c_prefix=c_var(field_prefix), prefix=field_prefix,
> +                             type=type_name(argentry), c_name=c_var(argname),
> +                             name=argname)
>  
>          if optional:
>              pop_indent()
>

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 122b4cb..a347203 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -110,12 +110,16 @@  def parse_args(typeinfo):
         argentry = typeinfo[member]
         optional = False
         structured = False
+        annotated = False
         if member.startswith('*'):
             argname = member[1:]
             optional = True
         if isinstance(argentry, OrderedDict):
-            structured = True
-        yield (argname, argentry, optional, structured)
+            if argentry.has_key('<annotated>'):
+                annotated = True
+            else:
+                structured = True
+        yield (argname, argentry, optional, structured, annotated)
 
 def de_camel_case(name):
     new_name = ''
diff --git a/scripts/qapi_commands.py b/scripts/qapi_commands.py
index 3c4678d..a2e0c23 100644
--- a/scripts/qapi_commands.py
+++ b/scripts/qapi_commands.py
@@ -32,7 +32,7 @@  void %(visitor)s(Visitor *m, %(name)s * obj, const char *name, Error **errp);
 
 def generate_command_decl(name, args, ret_type):
     arglist=""
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional, structured, annotated in parse_args(args):
         argtype = c_type(argtype)
         if argtype == "char *":
             argtype = "const char *"
@@ -50,7 +50,7 @@  def gen_sync_call(name, args, ret_type, indent=0):
     retval=""
     if ret_type:
         retval = "retval = "
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional, structured, annotated in parse_args(args):
         if optional:
             arglist += "has_%s, " % c_var(argname)
         arglist += "%s, " % (c_var(argname))
@@ -106,7 +106,7 @@  Visitor *v;
 def gen_visitor_input_vars_decl(args):
     ret = ""
     push_indent()
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional, structured, annotated in parse_args(args):
         if optional:
             ret += mcgen('''
 bool has_%(argname)s = false;
@@ -145,7 +145,7 @@  v = qmp_input_get_visitor(mi);
 ''',
                      obj=obj)
 
-    for argname, argtype, optional, structured in parse_args(args):
+    for argname, argtype, optional, structured, annotated in parse_args(args):
         if optional:
             ret += mcgen('''
 visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp);
diff --git a/scripts/qapi_types.py b/scripts/qapi_types.py
index 49ef569..6518768 100644
--- a/scripts/qapi_types.py
+++ b/scripts/qapi_types.py
@@ -45,7 +45,7 @@  struct %(name)s
 ''',
           name=structname)
 
-    for argname, argentry, optional, structured in parse_args(members):
+    for argname, argentry, optional, structured, annotated in parse_args(members):
         if optional:
             ret += mcgen('''
     bool has_%(c_name)s;
diff --git a/scripts/qapi_visit.py b/scripts/qapi_visit.py
index 974e458..b3f34a2 100644
--- a/scripts/qapi_visit.py
+++ b/scripts/qapi_visit.py
@@ -16,6 +16,52 @@  import sys
 import os
 import getopt
 import errno
+import types
+
+def generate_visit_carray_body(name, info):
+    if info['array_size'][0].isdigit():
+        array_size = info['array_size']
+    elif info['array_size'][0] == '(' and info['array_size'][-1] == ')':
+        array_size = info['array_size']
+    else:
+        array_size = "(*obj)->%s" % info['array_size']
+
+    if info.has_key('array_capacity'):
+        array_capacity = info['array_capacity']
+    else:
+        array_capacity = array_size
+
+    if camel_case(de_camel_case(info['type'][0])) == info['type'][0]:
+        ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+    %(type)s %(name)s_ptr = &(*obj)->%(name)s[%(name)s_i];
+    visit_next_carray(m, errp);
+    visit_type_%(type_short)s(m, &%(name)s_ptr, NULL, errp);
+}
+visit_end_carray(m, errp);
+''',
+                    name=name, type=c_type(info['type'][0]),
+                    type_short=info['type'][0],
+                    array_size=array_size,
+                    array_capacity=array_capacity)
+    else:
+        ret = mcgen('''
+visit_start_carray(m, (void **)obj, "%(name)s", %(array_capacity)s, sizeof(%(type)s), errp);
+int %(name)s_i;
+for (%(name)s_i = 0; %(name)s_i < %(array_size)s; %(name)s_i++) {
+    %(type)s *%(name)s_ptr = (%(type)s *)&(*obj)->%(name)s[%(name)s_i];
+    visit_next_carray(m, errp);
+    visit_type_%(type_short)s(m, %(name)s_ptr, NULL, errp);
+}
+visit_end_carray(m, errp);
+''',
+                    name=name, type=c_type(info['type'][0]),
+                    type_short=info['type'][0],
+                    array_size=array_size,
+                    array_capacity=array_capacity)
+    return ret
 
 def generate_visit_struct_body(field_prefix, name, members):
     ret = mcgen('''
@@ -45,10 +91,10 @@  if (!err) {
 
     push_indent()
     push_indent()
-    for argname, argentry, optional, structured in parse_args(members):
+    for argname, argentry, optional, structured, annotated in parse_args(members):
         if optional:
             ret += mcgen('''
-visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", &err);
+visit_start_optional(m, obj ? &(*obj)->%(c_prefix)shas_%(c_name)s : NULL, "%(name)s", errp);
 if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
 ''',
                          c_prefix=c_var(field_prefix), prefix=field_prefix,
@@ -58,12 +104,16 @@  if (obj && (*obj)->%(prefix)shas_%(c_name)s) {
         if structured:
             ret += generate_visit_struct_body(field_prefix + argname, argname, argentry)
         else:
-            ret += mcgen('''
-visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", &err);
+            if annotated:
+                if isinstance(argentry['type'], types.ListType):
+                    ret += generate_visit_carray_body(argname, argentry)
+            else:
+                ret += mcgen('''
+visit_type_%(type)s(m, obj ? &(*obj)->%(c_prefix)s%(c_name)s : NULL, "%(name)s", errp);
 ''',
-                         c_prefix=c_var(field_prefix), prefix=field_prefix,
-                         type=type_name(argentry), c_name=c_var(argname),
-                         name=argname)
+                             c_prefix=c_var(field_prefix), prefix=field_prefix,
+                             type=type_name(argentry), c_name=c_var(argname),
+                             name=argname)
 
         if optional:
             pop_indent()