[Qemu-devel] [PATCH v2 00/10] qapi: add support for lists of native types

[Qemu-devel] [PATCH v2 00/10] qapi: add support for lists of native types

[Michael Roth]

These patches apply on top of qemu.git master, and can also be obtained from:
git://github.com/mdroth/qemu.git qapi-native-lists

Sending this now since a number of series have popped up in the past that
wanted this, and Amos has some pending patches (query-mac-tables) that rely
on this as well.

These patches add support for specifying lists of native qapi types
(int/bool/str/number) like so:

  { 'type': 'foo',
    'data': { 'bar': ['int'] }}

for a 'bar' field that is a list of type 'int',

  { 'type': 'foo2',
    'data': { 'bar2': ['str'] }}

for a 'bar2' field that is a list of type 'str', and so on.

This uses linked list types for the native C representations, just as we do
for complex schema-defined types. In the future we may add schema annotations
of some sort to specify a more natural/efficient array type for the C
representations, but this should serve the majority of uses-cases for now.

v1->v2:
 * fixed do-nothing float tests in pre-existing code and updated new
   unit tests accordingly (Laszlo)
 * added a fix for a bug in json parser that was exposed by above change
 * fixed misuse of string format parameters for float testing (Laszlo)
 * fixed extra characters in comment (Laszlo)
 * removed unused variant from UserDefNativeListUnion QAPI type

 Makefile                           |    6 +-
 qapi-schema-test.json              |    7 +
 qobject/json-parser.c              |   26 +++-
 scripts/qapi-types.py              |   43 +++++-
 scripts/qapi-visit.py              |   36 ++++-
 scripts/qapi.py                    |   21 +++
 tests/test-qmp-input-visitor.c     |  186 ++++++++++++++++++++++++
 tests/test-qmp-output-visitor.c    |  176 ++++++++++++++++++++++
 tests/test-visitor-serialization.c |  282 +++++++++++++++++++++++++++++++-----
 9 files changed, 731 insertions(+), 52 deletions(-)

[Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support

[Michael Roth]

Teach type generators about native types so they can generate the
appropriate linked list types.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi-types.py |   43 ++++++++++++++++++++++++++++++++++++++++---
 scripts/qapi.py       |   21 +++++++++++++++++++++
 2 files changed, 61 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9e19920..96cb26d 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -16,7 +16,18 @@ import os
 import getopt
 import errno
 
-def generate_fwd_struct(name, members):
+def generate_fwd_struct(name, members, builtin_type=False):
+    if builtin_type:
+        return mcgen('''
+typedef struct %(name)sList
+{
+    %(type)s value;
+    struct %(name)sList *next;
+} %(name)sList;
+''',
+                     type=c_type(name),
+                     name=name)
+
     return mcgen('''
 typedef struct %(name)s %(name)s;
 
@@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj);
 
 def generate_type_cleanup(name):
     ret = mcgen('''
+
 void qapi_free_%(type)s(%(c_type)s obj)
 {
     QapiDeallocVisitor *md;
@@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj)
 
 
 try:
-    opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
-                                   ["source", "header", "prefix=", "output-dir="])
+    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
+                                   ["source", "header", "builtins",
+                                    "prefix=", "output-dir="])
 except getopt.GetoptError, err:
     print str(err)
     sys.exit(1)
@@ -197,6 +210,7 @@ h_file = 'qapi-types.h'
 
 do_c = False
 do_h = False
+do_builtins = False
 
 for o, a in opts:
     if o in ("-p", "--prefix"):
@@ -207,6 +221,8 @@ for o, a in opts:
         do_c = True
     elif o in ("-h", "--header"):
         do_h = True
+    elif o in ("-b", "--builtins"):
+        do_builtins = True
 
 if not do_c and not do_h:
     do_c = True
@@ -282,6 +298,11 @@ fdecl.write(mcgen('''
 exprs = parse_schema(sys.stdin)
 exprs = filter(lambda expr: not expr.has_key('gen'), exprs)
 
+fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
+for typename in builtin_types:
+    fdecl.write(generate_fwd_struct(typename, None, builtin_type=True))
+fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
+
 for expr in exprs:
     ret = "\n"
     if expr.has_key('type'):
@@ -298,6 +319,22 @@ for expr in exprs:
         continue
     fdecl.write(ret)
 
+# to avoid header dependency hell, we always generate declarations
+# for built-in types in our header files and simply guard them
+fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
+for typename in builtin_types:
+    fdecl.write(generate_type_cleanup_decl(typename + "List"))
+fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
+
+# ...this doesn't work for cases where we link in multiple objects that
+# have the functions defined, so we use -b option to provide control
+# over these cases
+if do_builtins:
+    fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
+    for typename in builtin_types:
+        fdef.write(generate_type_cleanup(typename + "List"))
+    fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
+
 for expr in exprs:
     ret = "\n"
     if expr.has_key('type'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index afc5f32..0ac8c2b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,6 +11,10 @@
 
 from ordereddict import OrderedDict
 
+builtin_types = [
+    'str', 'int', 'number', 'bool'
+]
+
 def tokenize(data):
     while len(data):
         ch = data[0]
@@ -242,3 +246,20 @@ def guardname(filename):
     for substr in [".", " ", "-"]:
         guard = guard.replace(substr, "_")
     return guard.upper() + '_H'
+
+def guardstart(name):
+    return mcgen('''
+
+#ifndef %(name)s
+#define %(name)s
+
+''',
+                 name=guardname(name))
+
+def guardend(name):
+    return mcgen('''
+
+#endif /* %(name)s */
+
+''',
+                 name=guardname(name))
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support

[Amos Kong]

On Thu, May 09, 2013 at 09:20:53PM -0500, Michael Roth wrote:
> Teach type generators about native types so they can generate the
> appropriate linked list types.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  scripts/qapi-types.py |   43 ++++++++++++++++++++++++++++++++++++++++---
>  scripts/qapi.py       |   21 +++++++++++++++++++++
>  2 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9e19920..96cb26d 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -16,7 +16,18 @@ import os
>  import getopt
>  import errno

<snip>

> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index afc5f32..0ac8c2b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -11,6 +11,10 @@
>  
>  from ordereddict import OrderedDict
>  
> +builtin_types = [
> +    'str', 'int', 'number', 'bool'

Will we add size, int8, uint32, ... in future when they are needed?

> +]
> +

-- 
			Amos.

Re: [Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support

[mdroth]

On Fri, May 10, 2013 at 11:04:22AM +0800, Amos Kong wrote:
> On Thu, May 09, 2013 at 09:20:53PM -0500, Michael Roth wrote:
> > Teach type generators about native types so they can generate the
> > appropriate linked list types.
> > 
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  scripts/qapi-types.py |   43 ++++++++++++++++++++++++++++++++++++++++---
> >  scripts/qapi.py       |   21 +++++++++++++++++++++
> >  2 files changed, 61 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > index 9e19920..96cb26d 100644
> > --- a/scripts/qapi-types.py
> > +++ b/scripts/qapi-types.py
> > @@ -16,7 +16,18 @@ import os
> >  import getopt
> >  import errno
> 
> <snip>
> 
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index afc5f32..0ac8c2b 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -11,6 +11,10 @@
> >  
> >  from ordereddict import OrderedDict
> >  
> > +builtin_types = [
> > +    'str', 'int', 'number', 'bool'
> 
> Will we add size, int8, uint32, ... in future when they are needed?

Well, that was the plan, but I hadn't recalled that support for these
was already been added to the code generators as part of the Netdev
opts-visitor stuff, so I'll go ahead and do a re-spin with those added.

> 
> > +]
> > +
> 
> -- 
> 			Amos.
> 

Re: [Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support

[Luiz Capitulino]

On Thu,  9 May 2013 21:20:53 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Teach type generators about native types so they can generate the
> appropriate linked list types.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  scripts/qapi-types.py |   43 ++++++++++++++++++++++++++++++++++++++++---
>  scripts/qapi.py       |   21 +++++++++++++++++++++
>  2 files changed, 61 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 9e19920..96cb26d 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -16,7 +16,18 @@ import os
>  import getopt
>  import errno
>  
> -def generate_fwd_struct(name, members):
> +def generate_fwd_struct(name, members, builtin_type=False):
> +    if builtin_type:
> +        return mcgen('''
> +typedef struct %(name)sList
> +{
> +    %(type)s value;
> +    struct %(name)sList *next;
> +} %(name)sList;
> +''',

Sorry for the utterly minor comment, but as you're going to respin please
add a newline before ''' so that we get the declarations properly separated
when generated.

> +                     type=c_type(name),
> +                     name=name)
> +
>      return mcgen('''
>  typedef struct %(name)s %(name)s;
>  
> @@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj);
>  
>  def generate_type_cleanup(name):
>      ret = mcgen('''
> +
>  void qapi_free_%(type)s(%(c_type)s obj)
>  {
>      QapiDeallocVisitor *md;
> @@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj)
>  
>  
>  try:
> -    opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
> -                                   ["source", "header", "prefix=", "output-dir="])
> +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> +                                   ["source", "header", "builtins",
> +                                    "prefix=", "output-dir="])
>  except getopt.GetoptError, err:
>      print str(err)
>      sys.exit(1)
> @@ -197,6 +210,7 @@ h_file = 'qapi-types.h'
>  
>  do_c = False
>  do_h = False
> +do_builtins = False
>  
>  for o, a in opts:
>      if o in ("-p", "--prefix"):
> @@ -207,6 +221,8 @@ for o, a in opts:
>          do_c = True
>      elif o in ("-h", "--header"):
>          do_h = True
> +    elif o in ("-b", "--builtins"):
> +        do_builtins = True
>  
>  if not do_c and not do_h:
>      do_c = True
> @@ -282,6 +298,11 @@ fdecl.write(mcgen('''
>  exprs = parse_schema(sys.stdin)
>  exprs = filter(lambda expr: not expr.has_key('gen'), exprs)
>  
> +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
> +for typename in builtin_types:
> +    fdecl.write(generate_fwd_struct(typename, None, builtin_type=True))
> +fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
> +
>  for expr in exprs:
>      ret = "\n"
>      if expr.has_key('type'):
> @@ -298,6 +319,22 @@ for expr in exprs:
>          continue
>      fdecl.write(ret)
>  
> +# to avoid header dependency hell, we always generate declarations
> +# for built-in types in our header files and simply guard them
> +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
> +for typename in builtin_types:
> +    fdecl.write(generate_type_cleanup_decl(typename + "List"))
> +fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))

I'm not sure I got why you're doing this. Is it because you're going to
generate them in more .h files? This is a bit ugly :(

> +
> +# ...this doesn't work for cases where we link in multiple objects that
> +# have the functions defined, so we use -b option to provide control
> +# over these cases
> +if do_builtins:
> +    fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
> +    for typename in builtin_types:
> +        fdef.write(generate_type_cleanup(typename + "List"))
> +    fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
> +
>  for expr in exprs:
>      ret = "\n"
>      if expr.has_key('type'):
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index afc5f32..0ac8c2b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -11,6 +11,10 @@
>  
>  from ordereddict import OrderedDict
>  
> +builtin_types = [
> +    'str', 'int', 'number', 'bool'
> +]
> +
>  def tokenize(data):
>      while len(data):
>          ch = data[0]
> @@ -242,3 +246,20 @@ def guardname(filename):
>      for substr in [".", " ", "-"]:
>          guard = guard.replace(substr, "_")
>      return guard.upper() + '_H'
> +
> +def guardstart(name):
> +    return mcgen('''
> +
> +#ifndef %(name)s
> +#define %(name)s
> +
> +''',
> +                 name=guardname(name))
> +
> +def guardend(name):
> +    return mcgen('''
> +
> +#endif /* %(name)s */
> +
> +''',
> +                 name=guardname(name))

Re: [Qemu-devel] [PATCH 01/10] qapi: qapi-types.py, native list support

[mdroth]

On Fri, May 10, 2013 at 10:07:45AM -0400, Luiz Capitulino wrote:
> On Thu,  9 May 2013 21:20:53 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > Teach type generators about native types so they can generate the
> > appropriate linked list types.
> > 
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  scripts/qapi-types.py |   43 ++++++++++++++++++++++++++++++++++++++++---
> >  scripts/qapi.py       |   21 +++++++++++++++++++++
> >  2 files changed, 61 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > index 9e19920..96cb26d 100644
> > --- a/scripts/qapi-types.py
> > +++ b/scripts/qapi-types.py
> > @@ -16,7 +16,18 @@ import os
> >  import getopt
> >  import errno
> >  
> > -def generate_fwd_struct(name, members):
> > +def generate_fwd_struct(name, members, builtin_type=False):
> > +    if builtin_type:
> > +        return mcgen('''
> > +typedef struct %(name)sList
> > +{
> > +    %(type)s value;
> > +    struct %(name)sList *next;
> > +} %(name)sList;
> > +''',
> 
> Sorry for the utterly minor comment, but as you're going to respin please
> add a newline before ''' so that we get the declarations properly separated
> when generated.
> 
> > +                     type=c_type(name),
> > +                     name=name)
> > +
> >      return mcgen('''
> >  typedef struct %(name)s %(name)s;
> >  
> > @@ -164,6 +175,7 @@ void qapi_free_%(type)s(%(c_type)s obj);
> >  
> >  def generate_type_cleanup(name):
> >      ret = mcgen('''
> > +
> >  void qapi_free_%(type)s(%(c_type)s obj)
> >  {
> >      QapiDeallocVisitor *md;
> > @@ -184,8 +196,9 @@ void qapi_free_%(type)s(%(c_type)s obj)
> >  
> >  
> >  try:
> > -    opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
> > -                                   ["source", "header", "prefix=", "output-dir="])
> > +    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
> > +                                   ["source", "header", "builtins",
> > +                                    "prefix=", "output-dir="])
> >  except getopt.GetoptError, err:
> >      print str(err)
> >      sys.exit(1)
> > @@ -197,6 +210,7 @@ h_file = 'qapi-types.h'
> >  
> >  do_c = False
> >  do_h = False
> > +do_builtins = False
> >  
> >  for o, a in opts:
> >      if o in ("-p", "--prefix"):
> > @@ -207,6 +221,8 @@ for o, a in opts:
> >          do_c = True
> >      elif o in ("-h", "--header"):
> >          do_h = True
> > +    elif o in ("-b", "--builtins"):
> > +        do_builtins = True
> >  
> >  if not do_c and not do_h:
> >      do_c = True
> > @@ -282,6 +298,11 @@ fdecl.write(mcgen('''
> >  exprs = parse_schema(sys.stdin)
> >  exprs = filter(lambda expr: not expr.has_key('gen'), exprs)
> >  
> > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
> > +for typename in builtin_types:
> > +    fdecl.write(generate_fwd_struct(typename, None, builtin_type=True))
> > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
> > +
> >  for expr in exprs:
> >      ret = "\n"
> >      if expr.has_key('type'):
> > @@ -298,6 +319,22 @@ for expr in exprs:
> >          continue
> >      fdecl.write(ret)
> >  
> > +# to avoid header dependency hell, we always generate declarations
> > +# for built-in types in our header files and simply guard them
> > +fdecl.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
> > +for typename in builtin_types:
> > +    fdecl.write(generate_type_cleanup_decl(typename + "List"))
> > +fdecl.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DECL"))
> 
> I'm not sure I got why you're doing this. Is it because you're going to
> generate them in more .h files? This is a bit ugly :(
> 

The issue is that things like the types generated from
qapi-schema-test.json or qga/qapi-schema.json may end up referencing
intList/strList/visit_type_intList/etc, which we'll also have declared for
use in the main qapi-schema.json. qapi-schema.json of course can't
depend on tests or qga, so we generate the definitions for the builtin types
when running the code generators on qapi-schema.json.

For everyone else, tests/qga/etc, to be able to use/link against those
definitions we need to include the declarations by either #include'ing
qapi-types.h/qapi-visit.h etc, or by always declaring them and simply
adding a guard.

In this case I've taken the latter approach since hard-coding a
reference in the code generators to header files created by separate calls
to the code generators seemed less modular. qapi-schema-test.json should
be capable of generating self-contained code if need be (and the only
reason we don't make it self-contained is that test-cases rely on
libqemuutil to build, which actually does have a dependency on
qapi-schema.json due to qemu-sockets.

> > +
> > +# ...this doesn't work for cases where we link in multiple objects that
> > +# have the functions defined, so we use -b option to provide control
> > +# over these cases
> > +if do_builtins:
> > +    fdef.write(guardstart("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
> > +    for typename in builtin_types:
> > +        fdef.write(generate_type_cleanup(typename + "List"))
> > +    fdef.write(guardend("QAPI_TYPES_BUILTIN_CLEANUP_DEF"))
> > +
> >  for expr in exprs:
> >      ret = "\n"
> >      if expr.has_key('type'):
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index afc5f32..0ac8c2b 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -11,6 +11,10 @@
> >  
> >  from ordereddict import OrderedDict
> >  
> > +builtin_types = [
> > +    'str', 'int', 'number', 'bool'
> > +]
> > +
> >  def tokenize(data):
> >      while len(data):
> >          ch = data[0]
> > @@ -242,3 +246,20 @@ def guardname(filename):
> >      for substr in [".", " ", "-"]:
> >          guard = guard.replace(substr, "_")
> >      return guard.upper() + '_H'
> > +
> > +def guardstart(name):
> > +    return mcgen('''
> > +
> > +#ifndef %(name)s
> > +#define %(name)s
> > +
> > +''',
> > +                 name=guardname(name))
> > +
> > +def guardend(name):
> > +    return mcgen('''
> > +
> > +#endif /* %(name)s */
> > +
> > +''',
> > +                 name=guardname(name))
> 

[Qemu-devel] [PATCH 02/10] qapi: qapi-visit.py, fix list handling for union types

[Michael Roth]

Currently we assume non-list types when generating visitor routines for
union types. This is broken, since values like ['Type'] need to mapped
to 'TypeList'.

We already have a type_name() function to handle this that we use for
generating struct visitors, so use that here as well.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi-visit.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index a276540..4c4de4b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -174,7 +174,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 ''',
                 abbrev = de_camel_case(name).upper(),
                 enum = c_fun(de_camel_case(key),False).upper(),
-                c_type=members[key],
+                c_type=type_name(members[key]),
                 c_name=c_fun(key))
 
     ret += mcgen('''
-- 
1.7.9.5

[Qemu-devel] [PATCH 03/10] qapi: qapi-visit.py, native list support

[Michael Roth]

Teach visitor generators about native types so they can generate the
appropriate visitor routines.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 scripts/qapi-visit.py |   34 +++++++++++++++++++++++++++++-----
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 4c4de4b..6cac05a 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -202,12 +202,14 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
 
     return ret
 
-def generate_declaration(name, members, genlist=True):
-    ret = mcgen('''
+def generate_declaration(name, members, genlist=True, builtin_type=False):
+    ret = ""
+    if not builtin_type:
+        ret += mcgen('''
 
 void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **errp);
 ''',
-                name=name)
+                    name=name)
 
     if genlist:
         ret += mcgen('''
@@ -235,8 +237,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e
                 name=name)
 
 try:
-    opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:",
-                                   ["source", "header", "prefix=", "output-dir="])
+    opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:",
+                                   ["source", "header", "builtins", "prefix=",
+                                    "output-dir="])
 except getopt.GetoptError, err:
     print str(err)
     sys.exit(1)
@@ -248,6 +251,7 @@ h_file = 'qapi-visit.h'
 
 do_c = False
 do_h = False
+do_builtins = False
 
 for o, a in opts:
     if o in ("-p", "--prefix"):
@@ -258,6 +262,8 @@ for o, a in opts:
         do_c = True
     elif o in ("-h", "--header"):
         do_h = True
+    elif o in ("-b", "--builtins"):
+        do_builtins = True
 
 if not do_c and not do_h:
     do_c = True
@@ -324,11 +330,29 @@ fdecl.write(mcgen('''
 
 #include "qapi/visitor.h"
 #include "%(prefix)sqapi-types.h"
+
 ''',
                   prefix=prefix, guard=guardname(h_file)))
 
 exprs = parse_schema(sys.stdin)
 
+# to avoid header dependency hell, we always generate declarations
+# for built-in types in our header files and simply guard them
+fdecl.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
+for typename in builtin_types:
+    fdecl.write(generate_declaration(typename, None, genlist=True,
+                                     builtin_type=True))
+fdecl.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DECL"))
+
+# ...this doesn't work for cases where we link in multiple objects that
+# have the functions defined, so we use -b option to provide control
+# over these cases
+if do_builtins:
+    fdef.write(guardstart("QAPI_VISIT_BUILTIN_VISITOR_DEF"))
+    for typename in builtin_types:
+        fdef.write(generate_visit_list(typename, None))
+    fdef.write(guardend("QAPI_VISIT_BUILTIN_VISITOR_DEF"))
+
 for expr in exprs:
     if expr.has_key('type'):
         ret = generate_visit_struct(expr['type'], expr['data'])
-- 
1.7.9.5

[Qemu-devel] [PATCH 04/10] qapi: enable generation of native list code

[Michael Roth]

Also, fix a dependency issue with libqemuutil: qemu-sockets.c needs
qapi-types.c/qapi-visit.c

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 Makefile |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 7dc0204..9695c9d 100644
--- a/Makefile
+++ b/Makefile
@@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 # Build libraries
 
 libqemustub.a: $(stub-obj-y)
-libqemuutil.a: $(util-obj-y)
+libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
 
 ######################################################################
 
@@ -215,10 +215,10 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 
 qapi-types.c qapi-types.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
-	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, "  GEN   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
 qapi-visit.c qapi-visit.h :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
-	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "."  < $<, "  GEN   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
 qmp-commands.h qmp-marshal.c :\
 $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
 	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, "  GEN   $@")
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 04/10] qapi: enable generation of native list code

[Luiz Capitulino]

On Thu,  9 May 2013 21:20:56 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Also, fix a dependency issue with libqemuutil: qemu-sockets.c needs
> qapi-types.c/qapi-visit.c
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  Makefile |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 7dc0204..9695c9d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
>  # Build libraries
>  
>  libqemustub.a: $(stub-obj-y)
> -libqemuutil.a: $(util-obj-y)
> +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o

Don't we want this in for 1.5?

>  
>  ######################################################################
>  
> @@ -215,10 +215,10 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>  
>  qapi-types.c qapi-types.h :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, "  GEN   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
>  qapi-visit.c qapi-visit.h :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "."  < $<, "  GEN   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
>  qmp-commands.h qmp-marshal.c :\
>  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
>  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, "  GEN   $@")

Re: [Qemu-devel] [PATCH 04/10] qapi: enable generation of native list code

[mdroth]

On Fri, May 10, 2013 at 10:10:03AM -0400, Luiz Capitulino wrote:
> On Thu,  9 May 2013 21:20:56 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > Also, fix a dependency issue with libqemuutil: qemu-sockets.c needs
> > qapi-types.c/qapi-visit.c
> > 
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  Makefile |    6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index 7dc0204..9695c9d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> >  # Build libraries
> >  
> >  libqemustub.a: $(stub-obj-y)
> > -libqemuutil.a: $(util-obj-y)
> > +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> 
> Don't we want this in for 1.5?
> 

I don't think it's causing any issues currently since it's not causing
undefined reference errors upstream. users of libqemuutil that make use
of qemu-sockets seem to be pulling qapi-types/qapi-visit in through other
dependencies.

I only noticed it because I was attempting to generate the native list
code via tests/Makefile and running into redefinition conflicts with
qapi-types.o/qapi-visit.o, then noticed the qemu-sockets.o issue when I
attempted to remove the qapi-types/qapi-visit dependency from
tests/test-visitor-serialization

Now that we're generating the native list code from top-level Makefile,
it actually doesn't seem to be needed by this series anymore, so maybe
I'll pull it out for now. I think a better fix would be to have
qapi/Makefile.obj add these to $util-obj-y directly anyway.

> >  
> >  ######################################################################
> >  
> > @@ -215,10 +215,10 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> >  
> >  qapi-types.c qapi-types.h :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> > -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, "  GEN   $@")
> > +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> >  qapi-visit.c qapi-visit.h :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> > -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "."  < $<, "  GEN   $@")
> > +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> >  qmp-commands.h qmp-marshal.c :\
> >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> >  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> 

Re: [Qemu-devel] [PATCH 04/10] qapi: enable generation of native list code

[mdroth]

On Fri, May 10, 2013 at 11:32:48AM -0500, mdroth wrote:
> On Fri, May 10, 2013 at 10:10:03AM -0400, Luiz Capitulino wrote:
> > On Thu,  9 May 2013 21:20:56 -0500
> > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > 
> > > Also, fix a dependency issue with libqemuutil: qemu-sockets.c needs
> > > qapi-types.c/qapi-visit.c
> > > 
> > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > > ---
> > >  Makefile |    6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/Makefile b/Makefile
> > > index 7dc0204..9695c9d 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -178,7 +178,7 @@ Makefile: $(version-obj-y) $(version-lobj-y)
> > >  # Build libraries
> > >  
> > >  libqemustub.a: $(stub-obj-y)
> > > -libqemuutil.a: $(util-obj-y)
> > > +libqemuutil.a: $(util-obj-y) qapi-types.o qapi-visit.o
> > 
> > Don't we want this in for 1.5?
> > 
> 
> I don't think it's causing any issues currently since it's not causing
> undefined reference errors upstream. users of libqemuutil that make use
> of qemu-sockets seem to be pulling qapi-types/qapi-visit in through other
> dependencies.
> 
> I only noticed it because I was attempting to generate the native list
> code via tests/Makefile and running into redefinition conflicts with
> qapi-types.o/qapi-visit.o, then noticed the qemu-sockets.o issue when I
> attempted to remove the qapi-types/qapi-visit dependency from
> tests/test-visitor-serialization
> 
> Now that we're generating the native list code from top-level Makefile,
> it actually doesn't seem to be needed by this series anymore, so maybe
> I'll pull it out for now. I think a better fix would be to have
> qapi/Makefile.obj add these to $util-obj-y directly anyway.

^ this wasn't quite right, we do have a new dependency on
qapi-types/qapi-visit in the visitor tests for native list types, so
I've left this in place.

> 
> > >  
> > >  ######################################################################
> > >  
> > > @@ -215,10 +215,10 @@ $(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> > >  
> > >  qapi-types.c qapi-types.h :\
> > >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> > > -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." < $<, "  GEN   $@")
> > > +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> > >  qapi-visit.c qapi-visit.h :\
> > >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> > > -	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "."  < $<, "  GEN   $@")
> > > +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, "  GEN   $@")
> > >  qmp-commands.h qmp-marshal.c :\
> > >  $(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py)
> > >  	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, "  GEN   $@")
> > 

[Qemu-devel] [PATCH 05/10] qapi: fix leak in unit tests

[Michael Roth]

qmp_output_get_qobject() increments the qobject's reference count. Since
we currently pass this straight into qobject_to_json() so we can feed
the data into a QMP input visitor, we never actually free the underlying
qobject when qmp_output_visitor_cleanup() is called. This causes leaks
on all of the QMP serialization tests.

Fix this by holding a pointer to the qobject and decref'ing it before
returning from qmp_deserialize().

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/test-visitor-serialization.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index e84926f..8c8adac 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -657,11 +657,16 @@ static void qmp_deserialize(void **native_out, void *datap,
                             VisitorFunc visit, Error **errp)
 {
     QmpSerializeData *d = datap;
-    QString *output_json = qobject_to_json(qmp_output_get_qobject(d->qov));
-    QObject *obj = qobject_from_json(qstring_get_str(output_json));
+    QString *output_json;
+    QObject *obj_orig, *obj;
+
+    obj_orig = qmp_output_get_qobject(d->qov);
+    output_json = qobject_to_json(obj_orig);
+    obj = qobject_from_json(qstring_get_str(output_json));
 
     QDECREF(output_json);
     d->qiv = qmp_input_visitor_new(obj);
+    qobject_decref(obj_orig);
     qobject_decref(obj);
     visit(qmp_input_get_visitor(d->qiv), native_out, errp);
 }
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 05/10] qapi: fix leak in unit tests

[Luiz Capitulino]

On Thu,  9 May 2013 21:20:57 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> qmp_output_get_qobject() increments the qobject's reference count. Since
> we currently pass this straight into qobject_to_json() so we can feed
> the data into a QMP input visitor, we never actually free the underlying
> qobject when qmp_output_visitor_cleanup() is called. This causes leaks
> on all of the QMP serialization tests.
> 
> Fix this by holding a pointer to the qobject and decref'ing it before
> returning from qmp_deserialize().
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>

I've cherry-picked this one into the qmp branch for 1.5.

> ---
>  tests/test-visitor-serialization.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
> index e84926f..8c8adac 100644
> --- a/tests/test-visitor-serialization.c
> +++ b/tests/test-visitor-serialization.c
> @@ -657,11 +657,16 @@ static void qmp_deserialize(void **native_out, void *datap,
>                              VisitorFunc visit, Error **errp)
>  {
>      QmpSerializeData *d = datap;
> -    QString *output_json = qobject_to_json(qmp_output_get_qobject(d->qov));
> -    QObject *obj = qobject_from_json(qstring_get_str(output_json));
> +    QString *output_json;
> +    QObject *obj_orig, *obj;
> +
> +    obj_orig = qmp_output_get_qobject(d->qov);
> +    output_json = qobject_to_json(obj_orig);
> +    obj = qobject_from_json(qstring_get_str(output_json));
>  
>      QDECREF(output_json);
>      d->qiv = qmp_input_visitor_new(obj);
> +    qobject_decref(obj_orig);
>      qobject_decref(obj);
>      visit(qmp_input_get_visitor(d->qiv), native_out, errp);
>  }

[Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Michael Roth]

Currently our JSON parser assumes that numbers lacking a mantissa are
integers and attempts to store them as QInt/int64 values. This breaks in
the case where the number overflows/underflows int64 values (which is
still valid JSON)

Fix this by detecting such cases and using a QFloat to store the value
instead.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qobject/json-parser.c |   26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 05279c1..4d14e71 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -640,9 +640,29 @@ static QObject *parse_literal(JSONParserContext *ctxt)
     case JSON_STRING:
         obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
         break;
-    case JSON_INTEGER:
-        obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
-        break;
+    case JSON_INTEGER: {
+        /* A possibility exists that this is a whole-valued float where the
+         * mantissa was left out due to being 0 (.0). It's not a big deal to
+         * treat these as ints in the parser, so long as users of the
+         * resulting QObject know to expect a QInt in place of a QFloat in
+         * cases like these.
+         *
+         * However, in some cases these values will overflow/underflow a
+         * QInt/int64 container, thus we should assume these are to be handled
+         * as QFloats/doubles rather than silently changing their values.
+         *
+         * strtoll() indicates these instances by setting errno to ERANGE
+         */
+        int64_t value;
+
+        errno = 0; /* strtoll doesn't set errno on success */
+        value = strtoll(token_get_value(token), NULL, 10);
+        if (errno != ERANGE) {
+            obj = QOBJECT(qint_from_int(value));
+            break;
+        }
+        /* fall through to JSON_FLOAT */
+    }
     case JSON_FLOAT:
         /* FIXME dependent on locale */
         obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token), NULL)));
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Laszlo Ersek]

On 05/10/13 04:20, Michael Roth wrote:
> Currently our JSON parser assumes that numbers lacking a mantissa are
> integers and attempts to store them as QInt/int64 values. This breaks in
> the case where the number overflows/underflows int64 values (which is
> still valid JSON)
> 
> Fix this by detecting such cases and using a QFloat to store the value
> instead.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qobject/json-parser.c |   26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 05279c1..4d14e71 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -640,9 +640,29 @@ static QObject *parse_literal(JSONParserContext *ctxt)
>      case JSON_STRING:
>          obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
>          break;
> -    case JSON_INTEGER:
> -        obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> -        break;
> +    case JSON_INTEGER: {
> +        /* A possibility exists that this is a whole-valued float where the
> +         * mantissa was left out due to being 0 (.0). It's not a big deal to
> +         * treat these as ints in the parser, so long as users of the
> +         * resulting QObject know to expect a QInt in place of a QFloat in
> +         * cases like these.
> +         *
> +         * However, in some cases these values will overflow/underflow a
> +         * QInt/int64 container, thus we should assume these are to be handled
> +         * as QFloats/doubles rather than silently changing their values.
> +         *
> +         * strtoll() indicates these instances by setting errno to ERANGE
> +         */
> +        int64_t value;
> +
> +        errno = 0; /* strtoll doesn't set errno on success */
> +        value = strtoll(token_get_value(token), NULL, 10);
> +        if (errno != ERANGE) {
> +            obj = QOBJECT(qint_from_int(value));
> +            break;
> +        }
> +        /* fall through to JSON_FLOAT */
> +    }
>      case JSON_FLOAT:
>          /* FIXME dependent on locale */
>          obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token), NULL)));
> 

I wanted to correct you here and propose s/mantissa/fractional part/
everywhere in this patch (including commit message and patch body).

However <http://en.wiktionary.org/wiki/mantissa> tells me one meaning of
"mantissa" *is* "fractional part".

In that sense I won't try to "correct" you, but can you consider doing
the replacement still, in order not to confuse non-native speakers who
associate "mantissa" only with the third meaning Wiktionary gives, ie.
the significand in FP representation?

Otherwise the v2 series looks good to me (not R-b-ing it since you'll
post v3 to address Amos's note).

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]

On 05/09/2013 08:20 PM, Michael Roth wrote:
> Currently our JSON parser assumes that numbers lacking a mantissa are
> integers and attempts to store them as QInt/int64 values. This breaks in
> the case where the number overflows/underflows int64 values (which is
> still valid JSON)
> 
> Fix this by detecting such cases and using a QFloat to store the value
> instead.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qobject/json-parser.c |   26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)

This changes the error message handed back to QMP clients, and possibly
exposes problems in other qemu code that receives the result of json
parses.  Previously, for an 'int' argument, if you passed in a too-large
number, you got an error that the argument was too large for int.  Now,
the number is accepted as a double; are we guaranteed that in a context
that expects a qint, when that code is now handed a qfloat (a case which
was previously impossible because qint_from_int protected it), that the
code will still behave correctly?

At any rate, libvirt already checks that all numbers that fall outside
the range of int64_t are never passed over qmp when passing an int
argument (and yes, this is annoying, in that large 64-bit unsigned
numbers have to be passed as negative numbers, rather than exceeding
INT64_MAX), so libvirt should not be triggering this newly exposed code
path.  But even if libvirt doesn't plan on triggering it, I'd still feel
better if your commit message documented evidence of testing what
happens in this case.  For example, compare what
{"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
does before and after this patch.

> +        /* fall through to JSON_FLOAT */
> +    }
>      case JSON_FLOAT:
>          /* FIXME dependent on locale */

You know, strtoll() also is dependent on locale (that is, non-C locale
can accept strings that the C locale rejects), if you want to repeat
this comment earlier for case JSON_INTEGER.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Laszlo Ersek]

On 05/10/13 14:22, Eric Blake wrote:
> On 05/09/2013 08:20 PM, Michael Roth wrote:
>> Currently our JSON parser assumes that numbers lacking a mantissa are
>> integers and attempts to store them as QInt/int64 values. This breaks in
>> the case where the number overflows/underflows int64 values (which is
>> still valid JSON)
>>
>> Fix this by detecting such cases and using a QFloat to store the value
>> instead.
>>
>> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
>> ---
>>  qobject/json-parser.c |   26 +++++++++++++++++++++++---
>>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> This changes the error message handed back to QMP clients, and possibly
> exposes problems in other qemu code that receives the result of json
> parses.  Previously, for an 'int' argument, if you passed in a too-large
> number, you got an error that the argument was too large for int.  Now,
> the number is accepted as a double; are we guaranteed that in a context
> that expects a qint, when that code is now handed a qfloat (a case which
> was previously impossible because qint_from_int protected it), that the
> code will still behave correctly?

I tried to consider this while reviewing... Maybe I was wrong.

The pre-patch code for JSON_INTEGER:

obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));

doesn't check for errors at all. (I assume that JSON_INTEGER is selected
by the parser, token_get_type(), based on syntax purely.)

I thought when the pre-patch version encounters an int-looking decimal
string that's actually too big in magnitude for an int, you'd simply end
up with LLONG_MIN or LLONG_MAX, but no error. strtoll() clamps the
value, errno is lost, and qint_from_int() sees nothing wrong.

With the patch, you end up with a float instead of an int-typed
LLONG_MIN/LLONG_MAX, and also no error.

> At any rate, libvirt already checks that all numbers that fall outside
> the range of int64_t are never passed over qmp when passing an int
> argument (and yes, this is annoying, in that large 64-bit unsigned
> numbers have to be passed as negative numbers, rather than exceeding
> INT64_MAX), so libvirt should not be triggering this newly exposed code
> path.  But even if libvirt doesn't plan on triggering it, I'd still feel
> better if your commit message documented evidence of testing what
> happens in this case.  For example, compare what
> {"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
> does before and after this patch.

That would be likely interesting to test, yes.

Laszlo

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[mdroth]

On Fri, May 10, 2013 at 02:47:18PM +0200, Laszlo Ersek wrote:
> On 05/10/13 14:22, Eric Blake wrote:
> > On 05/09/2013 08:20 PM, Michael Roth wrote:
> >> Currently our JSON parser assumes that numbers lacking a mantissa are
> >> integers and attempts to store them as QInt/int64 values. This breaks in
> >> the case where the number overflows/underflows int64 values (which is
> >> still valid JSON)
> >>
> >> Fix this by detecting such cases and using a QFloat to store the value
> >> instead.
> >>
> >> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> >> ---
> >>  qobject/json-parser.c |   26 +++++++++++++++++++++++---
> >>  1 file changed, 23 insertions(+), 3 deletions(-)
> > 
> > This changes the error message handed back to QMP clients, and possibly
> > exposes problems in other qemu code that receives the result of json
> > parses.  Previously, for an 'int' argument, if you passed in a too-large
> > number, you got an error that the argument was too large for int.  Now,
> > the number is accepted as a double; are we guaranteed that in a context
> > that expects a qint, when that code is now handed a qfloat (a case which
> > was previously impossible because qint_from_int protected it), that the
> > code will still behave correctly?
> 
> I tried to consider this while reviewing... Maybe I was wrong.
> 
> The pre-patch code for JSON_INTEGER:
> 
> obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> 
> doesn't check for errors at all. (I assume that JSON_INTEGER is selected
> by the parser, token_get_type(), based on syntax purely.)
> 
> I thought when the pre-patch version encounters an int-looking decimal
> string that's actually too big in magnitude for an int, you'd simply end
> up with LLONG_MIN or LLONG_MAX, but no error. strtoll() clamps the
> value, errno is lost, and qint_from_int() sees nothing wrong.
> 
> With the patch, you end up with a float instead of an int-typed
> LLONG_MIN/LLONG_MAX, and also no error.
> 

This is correct, but it does make code acting on the resulting QObject
capable of checking for this scenario and acting accordingly in whatever
manner is appropriate. I think that's appropriate, since there's no error
as far as the JSON spec goes, but further up the stack we may have stricter
requirements on what the valid ranges are for various fields.

For instance, in the case of QmpInputVisitor, these cases will
trigger an error case that was previously being bypassed when
out-of-range values were provided in place of 'int' due to the parser
silently capping the max/min values:

in qmp_input_type_int():

    QObject *qobj = qmp_input_get_object(qiv, name);

    if (!qobj || qobject_type(qobj) != QTYPE_QINT) {
        error_set(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
                  "integer");
        return;
    }

We'd now trigger this error for such inputs (since we'd have QTYPE_QFLOAT),
which is stricter checking than before, but as intended since we map int to
int64_t and expect values in that range (though we could stand to be clearer
about this in the QAPI documentation)

> > At any rate, libvirt already checks that all numbers that fall outside
> > the range of int64_t are never passed over qmp when passing an int
> > argument (and yes, this is annoying, in that large 64-bit unsigned
> > numbers have to be passed as negative numbers, rather than exceeding
> > INT64_MAX), so libvirt should not be triggering this newly exposed code
> > path.  But even if libvirt doesn't plan on triggering it, I'd still feel
> > better if your commit message documented evidence of testing what
> > happens in this case.  For example, compare what
> > {"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
> > does before and after this patch.
> 
> That would be likely interesting to test, yes.

That would trigger the error mentioned above, as opposed to silently changing
the value to LLONG_MAX. I'll see about covering this case somewhere in
tests/test-qmp-input-visitor.

> 
> Laszlo
> 

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2659 bytes --]

On 05/10/2013 06:47 AM, Laszlo Ersek wrote:

> The pre-patch code for JSON_INTEGER:
> 
> obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> 
> doesn't check for errors at all. (I assume that JSON_INTEGER is selected
> by the parser, token_get_type(), based on syntax purely.)
> 
> I thought when the pre-patch version encounters an int-looking decimal
> string that's actually too big in magnitude for an int, you'd simply end
> up with LLONG_MIN or LLONG_MAX, but no error. strtoll() clamps the
> value, errno is lost, and qint_from_int() sees nothing wrong.

Oh, right.  _That's_ why libvirt had to add checks that it wasn't
passing 0x8000000000000000ULL as a positive number - because the qemu
parser was silently clamping it to 0x7fffffffffffffffLL, which is not
what libvirt wanted.  So the code was NOT erroring out with an overflow
message, but was acting on the wrong integer.

> 
> With the patch, you end up with a float instead of an int-typed
> LLONG_MIN/LLONG_MAX, and also no error.

Ah, but here we have a difference - beforehand, the code was passing a
valid (albeit wrong value) qint, so the rest of the qemu code was
oblivious to the fact that the QMP message contained an overflow.  But
now the code is passing a qdouble, and the rest of the qemu code may be
unprepared to handle it when expecting a qint.

> 
>> At any rate, libvirt already checks that all numbers that fall outside
>> the range of int64_t are never passed over qmp when passing an int
>> argument (and yes, this is annoying, in that large 64-bit unsigned
>> numbers have to be passed as negative numbers, rather than exceeding
>> INT64_MAX), so libvirt should not be triggering this newly exposed code
>> path.  But even if libvirt doesn't plan on triggering it, I'd still feel
>> better if your commit message documented evidence of testing what
>> happens in this case.  For example, compare what
>> {"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
>> does before and after this patch.
> 
> That would be likely interesting to test, yes.

add-fd may not be the best candidate (it expects an fd to be passed at
the same time, and does its own checking that it does not get a negative
number); but I'm sure there's plenty of other candidates (add-cpu is
another possibility that comes quickly to mind) - basically, pick a
command that takes an explicit 'int' argument, and overflow that
argument to see what happens when the command now has to deal with a
qdouble.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[mdroth]

On Fri, May 10, 2013 at 08:08:05AM -0600, Eric Blake wrote:
> On 05/10/2013 06:47 AM, Laszlo Ersek wrote:
> 
> > The pre-patch code for JSON_INTEGER:
> > 
> > obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> > 
> > doesn't check for errors at all. (I assume that JSON_INTEGER is selected
> > by the parser, token_get_type(), based on syntax purely.)
> > 
> > I thought when the pre-patch version encounters an int-looking decimal
> > string that's actually too big in magnitude for an int, you'd simply end
> > up with LLONG_MIN or LLONG_MAX, but no error. strtoll() clamps the
> > value, errno is lost, and qint_from_int() sees nothing wrong.
> 
> Oh, right.  _That's_ why libvirt had to add checks that it wasn't
> passing 0x8000000000000000ULL as a positive number - because the qemu
> parser was silently clamping it to 0x7fffffffffffffffLL, which is not
> what libvirt wanted.  So the code was NOT erroring out with an overflow
> message, but was acting on the wrong integer.
> 
> > 
> > With the patch, you end up with a float instead of an int-typed
> > LLONG_MIN/LLONG_MAX, and also no error.
> 
> Ah, but here we have a difference - beforehand, the code was passing a
> valid (albeit wrong value) qint, so the rest of the qemu code was
> oblivious to the fact that the QMP message contained an overflow.  But
> now the code is passing a qdouble, and the rest of the qemu code may be
> unprepared to handle it when expecting a qint.

Yup, new error cases can be triggered, but in the case of
QmpInputVisitor this is handled appropriately (will add a test case to
confirm), and none of our other input visitors act on QObjects, and this
ambiguity isn't present for output visitors.

We also have monitor events that call qobject_from_json() to marshall
event payloads, but these are essentially open-coded QmpInputVisitors
where the JSON values come from native C types. The only case where I
can see this triggering the change is if they did something like:

  obj = qobject_from_jsonf("{'myInt': %f}", whole_valued_float);

which would be evil, and thankfully such cases don't appear to exist:

mdroth@loki:~/w/qemu.git$ ack-grep qobject_from_json | grep "%f"
tests/check-qjson.c:987:    obj = qobject_from_jsonf("%f", valuef);
mdroth@loki:~/w/qemu.git$

(the 'valuef' above is not whole-valued, and the output is expected to
be a QFloat)

I'm not aware of any other cases to consider, but I might've missed
something.

> 
> > 
> >> At any rate, libvirt already checks that all numbers that fall outside
> >> the range of int64_t are never passed over qmp when passing an int
> >> argument (and yes, this is annoying, in that large 64-bit unsigned
> >> numbers have to be passed as negative numbers, rather than exceeding
> >> INT64_MAX), so libvirt should not be triggering this newly exposed code
> >> path.  But even if libvirt doesn't plan on triggering it, I'd still feel
> >> better if your commit message documented evidence of testing what
> >> happens in this case.  For example, compare what
> >> {"execute":"add-fd","arguments":{"fdset-id":"99999999999999999999"}}
> >> does before and after this patch.
> > 
> > That would be likely interesting to test, yes.
> 
> add-fd may not be the best candidate (it expects an fd to be passed at
> the same time, and does its own checking that it does not get a negative
> number); but I'm sure there's plenty of other candidates (add-cpu is
> another possibility that comes quickly to mind) - basically, pick a
> command that takes an explicit 'int' argument, and overflow that
> argument to see what happens when the command now has to deal with a
> qdouble.

Command params will end up getting marshalled in QObject prior to being
passed into commands:

    mi = qmp_input_visitor_new_strict(QOBJECT(args));
    v = qmp_input_get_visitor(mi);
    visit_start_optional(v, &has_fdset_id, "fdset-id", errp);
    if (has_fdset_id) {
        visit_type_int(v, &fdset_id, "fdset-id", errp);
    }
    visit_end_optional(v, errp);
    visit_start_optional(v, &has_opaque, "opaque", errp);
    if (has_opaque) {
        visit_type_str(v, &opaque, "opaque", errp);
    }
    visit_end_optional(v, errp);
    qmp_input_visitor_cleanup(mi);

    if (error_is_set(errp)) {
        goto out;
    }
    retval = qmp_add_fd(has_fdset_id, fdset_id, has_opaque, opaque, errp);

so i think a check in tests-qmp-input-visitor that verifies that values that
exceed LLONG_MAX/LLONG_MIN will get added into the QObject as QFloats
and trigger a type error when being passed to visit_type_int() should
cover the cases in question.

> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[Luiz Capitulino]

On Thu,  9 May 2013 21:20:58 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Currently our JSON parser assumes that numbers lacking a mantissa are
> integers and attempts to store them as QInt/int64 values. This breaks in
> the case where the number overflows/underflows int64 values (which is
> still valid JSON)

Anthony wanted to fix this by moving to another wire format :)

But, how this patch related to this series?

> 
> Fix this by detecting such cases and using a QFloat to store the value
> instead.
> 
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qobject/json-parser.c |   26 +++++++++++++++++++++++---
>  1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 05279c1..4d14e71 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -640,9 +640,29 @@ static QObject *parse_literal(JSONParserContext *ctxt)
>      case JSON_STRING:
>          obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
>          break;
> -    case JSON_INTEGER:
> -        obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> -        break;
> +    case JSON_INTEGER: {
> +        /* A possibility exists that this is a whole-valued float where the
> +         * mantissa was left out due to being 0 (.0). It's not a big deal to
> +         * treat these as ints in the parser, so long as users of the
> +         * resulting QObject know to expect a QInt in place of a QFloat in
> +         * cases like these.
> +         *
> +         * However, in some cases these values will overflow/underflow a
> +         * QInt/int64 container, thus we should assume these are to be handled
> +         * as QFloats/doubles rather than silently changing their values.
> +         *
> +         * strtoll() indicates these instances by setting errno to ERANGE
> +         */
> +        int64_t value;
> +
> +        errno = 0; /* strtoll doesn't set errno on success */
> +        value = strtoll(token_get_value(token), NULL, 10);
> +        if (errno != ERANGE) {
> +            obj = QOBJECT(qint_from_int(value));
> +            break;
> +        }
> +        /* fall through to JSON_FLOAT */
> +    }
>      case JSON_FLOAT:
>          /* FIXME dependent on locale */
>          obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token), NULL)));

Re: [Qemu-devel] [PATCH 06/10] json-parser: fix handling of large whole number values

[mdroth]

On Fri, May 10, 2013 at 11:17:17AM -0400, Luiz Capitulino wrote:
> On Thu,  9 May 2013 21:20:58 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > Currently our JSON parser assumes that numbers lacking a mantissa are
> > integers and attempts to store them as QInt/int64 values. This breaks in
> > the case where the number overflows/underflows int64 values (which is
> > still valid JSON)
> 
> Anthony wanted to fix this by moving to another wire format :)
> 
> But, how this patch related to this series?

In v1 Laszlo pointed out that the QFloat tests I added in
test-visitor-serialization were actually non-functional, and those tests were
based on pre-existing code. So I added a patch to fix the pre-existing
code as a pre-cursor to the new unit tests based on it. But fixing that code
exposed the json-parser bug, so I added that fix as a precursor to the
precursor :)

Same with mem leak fixes, etc, to ensure the tests were functional and
leak-free within this series.

> 
> > 
> > Fix this by detecting such cases and using a QFloat to store the value
> > instead.
> > 
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  qobject/json-parser.c |   26 +++++++++++++++++++++++---
> >  1 file changed, 23 insertions(+), 3 deletions(-)
> > 
> > diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> > index 05279c1..4d14e71 100644
> > --- a/qobject/json-parser.c
> > +++ b/qobject/json-parser.c
> > @@ -640,9 +640,29 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> >      case JSON_STRING:
> >          obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
> >          break;
> > -    case JSON_INTEGER:
> > -        obj = QOBJECT(qint_from_int(strtoll(token_get_value(token), NULL, 10)));
> > -        break;
> > +    case JSON_INTEGER: {
> > +        /* A possibility exists that this is a whole-valued float where the
> > +         * mantissa was left out due to being 0 (.0). It's not a big deal to
> > +         * treat these as ints in the parser, so long as users of the
> > +         * resulting QObject know to expect a QInt in place of a QFloat in
> > +         * cases like these.
> > +         *
> > +         * However, in some cases these values will overflow/underflow a
> > +         * QInt/int64 container, thus we should assume these are to be handled
> > +         * as QFloats/doubles rather than silently changing their values.
> > +         *
> > +         * strtoll() indicates these instances by setting errno to ERANGE
> > +         */
> > +        int64_t value;
> > +
> > +        errno = 0; /* strtoll doesn't set errno on success */
> > +        value = strtoll(token_get_value(token), NULL, 10);
> > +        if (errno != ERANGE) {
> > +            obj = QOBJECT(qint_from_int(value));
> > +            break;
> > +        }
> > +        /* fall through to JSON_FLOAT */
> > +    }
> >      case JSON_FLOAT:
> >          /* FIXME dependent on locale */
> >          obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token), NULL)));
> 

[Qemu-devel] [PATCH 07/10] qapi: fix visitor serialization tests for numbers/doubles

[Michael Roth]

We never actually stored the stringified double values into the strings
before we did the comparisons. This left number/double values completely
uncovered in test-visitor-serialization tests.

Fixing this exposed a bug in our handling of large whole number values
in QEMU's JSON parser which is now fixed.

Simplify the code while we're at it by dropping the
calc_float_string_storage() craziness in favor of GStrings.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/test-visitor-serialization.c |   25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 8c8adac..fed6810 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -229,17 +229,6 @@ typedef struct TestArgs {
     void *test_data;
 } TestArgs;
 
-#define FLOAT_STRING_PRECISION 6 /* corresponding to n in %.nf formatting */
-static gsize calc_float_string_storage(double value)
-{
-    int whole_value = value;
-    gsize i = 0;
-    do {
-        i++;
-    } while (whole_value /= 10);
-    return i + 2 + FLOAT_STRING_PRECISION;
-}
-
 static void test_primitives(gconstpointer opaque)
 {
     TestArgs *args = (TestArgs *) opaque;
@@ -248,7 +237,6 @@ static void test_primitives(gconstpointer opaque)
     PrimitiveType *pt_copy = g_malloc0(sizeof(*pt_copy));
     Error *err = NULL;
     void *serialize_data;
-    char *double1, *double2;
 
     pt_copy->type = pt->type;
     ops->serialize(pt, &serialize_data, visit_primitive_type, &err);
@@ -260,14 +248,17 @@ static void test_primitives(gconstpointer opaque)
         g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
         g_free((char *)pt_copy->value.string);
     } else if (pt->type == PTYPE_NUMBER) {
+        GString *double_expected = g_string_new("");
+        GString *double_actual = g_string_new("");
         /* we serialize with %f for our reference visitors, so rather than fuzzy
          * floating math to test "equality", just compare the formatted values
          */
-        double1 = g_malloc0(calc_float_string_storage(pt->value.number));
-        double2 = g_malloc0(calc_float_string_storage(pt_copy->value.number));
-        g_assert_cmpstr(double1, ==, double2);
-        g_free(double1);
-        g_free(double2);
+        g_string_printf(double_expected, "%.6f", pt->value.number);
+        g_string_printf(double_actual, "%.6f", pt_copy->value.number);
+        g_assert_cmpstr(double_actual->str, ==, double_expected->str);
+
+        g_string_free(double_expected, true);
+        g_string_free(double_actual, true);
     } else if (pt->type == PTYPE_BOOLEAN) {
         g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max);
     } else {
-- 
1.7.9.5

[Qemu-devel] [PATCH 08/10] qapi: add native list coverage for visitor serialization tests

[Michael Roth]

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/test-visitor-serialization.c |  248 +++++++++++++++++++++++++++++++++---
 1 file changed, 230 insertions(+), 18 deletions(-)

diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index fed6810..b31911f 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -23,6 +23,25 @@
 #include "qapi/qmp-output-visitor.h"
 #include "qapi/string-input-visitor.h"
 #include "qapi/string-output-visitor.h"
+#include "qapi-types.h"
+#include "qapi-visit.h"
+#include "qapi/dealloc-visitor.h"
+
+enum PrimitiveTypeKind {
+    PTYPE_STRING = 0,
+    PTYPE_BOOLEAN,
+    PTYPE_NUMBER,
+    PTYPE_INTEGER,
+    PTYPE_U8,
+    PTYPE_U16,
+    PTYPE_U32,
+    PTYPE_U64,
+    PTYPE_S8,
+    PTYPE_S16,
+    PTYPE_S32,
+    PTYPE_S64,
+    PTYPE_EOL,
+};
 
 typedef struct PrimitiveType {
     union {
@@ -40,26 +59,34 @@ typedef struct PrimitiveType {
         int64_t s64;
         intmax_t max;
     } value;
-    enum {
-        PTYPE_STRING = 0,
-        PTYPE_BOOLEAN,
-        PTYPE_NUMBER,
-        PTYPE_INTEGER,
-        PTYPE_U8,
-        PTYPE_U16,
-        PTYPE_U32,
-        PTYPE_U64,
-        PTYPE_S8,
-        PTYPE_S16,
-        PTYPE_S32,
-        PTYPE_S64,
-        PTYPE_EOL,
-    } type;
+    enum PrimitiveTypeKind type;
     const char *description;
 } PrimitiveType;
 
+typedef struct PrimitiveList {
+    union {
+        strList *strings;
+        boolList *booleans;
+        numberList *numbers;
+        intList *integers;
+    } value;
+    enum PrimitiveTypeKind type;
+    const char *description;
+} PrimitiveList;
+
 /* test helpers */
 
+typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp);
+
+static void dealloc_helper(void *native_in, VisitorFunc visit, Error **errp)
+{
+    QapiDeallocVisitor *qdv = qapi_dealloc_visitor_new();
+
+    visit(qapi_dealloc_get_visitor(qdv), &native_in, errp);
+
+    qapi_dealloc_visitor_cleanup(qdv);
+}
+
 static void visit_primitive_type(Visitor *v, void **native, Error **errp)
 {
     PrimitiveType *pt = *native;
@@ -105,6 +132,27 @@ static void visit_primitive_type(Visitor *v, void **native, Error **errp)
     }
 }
 
+static void visit_primitive_list(Visitor *v, void **native, Error **errp)
+{
+    PrimitiveList *pl = *native;
+    switch (pl->type) {
+    case PTYPE_STRING:
+        visit_type_strList(v, &pl->value.strings, NULL, errp);
+        break;
+    case PTYPE_BOOLEAN:
+        visit_type_boolList(v, &pl->value.booleans, NULL, errp);
+        break;
+    case PTYPE_NUMBER:
+        visit_type_numberList(v, &pl->value.numbers, NULL, errp);
+        break;
+    case PTYPE_INTEGER:
+        visit_type_intList(v, &pl->value.integers, NULL, errp);
+        break;
+    default:
+        g_assert(false);
+    }
+}
+
 typedef struct TestStruct
 {
     int64_t integer;
@@ -206,12 +254,11 @@ static void visit_nested_struct_list(Visitor *v, void **native, Error **errp)
 
 /* test cases */
 
-typedef void (*VisitorFunc)(Visitor *v, void **native, Error **errp);
-
 typedef enum VisitorCapabilities {
     VCAP_PRIMITIVES = 1,
     VCAP_STRUCTURES = 2,
     VCAP_LISTS = 4,
+    VCAP_PRIMITIVE_LISTS = 8,
 } VisitorCapabilities;
 
 typedef struct SerializeOps {
@@ -270,6 +317,152 @@ static void test_primitives(gconstpointer opaque)
     g_free(pt_copy);
 }
 
+static void test_primitive_lists(gconstpointer opaque)
+{
+    TestArgs *args = (TestArgs *) opaque;
+    const SerializeOps *ops = args->ops;
+    PrimitiveType *pt = args->test_data;
+    PrimitiveList pl = { .value = { 0 } };
+    PrimitiveList pl_copy = { .value = { 0 } };
+    PrimitiveList *pl_copy_ptr = &pl_copy;
+    Error *err = NULL;
+    void *serialize_data;
+    void *cur_head = NULL;
+    int i;
+
+    pl.type = pl_copy.type = pt->type;
+
+    /* build up our list of primitive types */
+    for (i = 0; i < 32; i++) {
+        switch (pl.type) {
+        case PTYPE_STRING: {
+            strList *tmp = g_new0(strList, 1);
+            tmp->value = g_strdup(pt->value.string);
+            if (pl.value.strings == NULL) {
+                pl.value.strings = tmp;
+            } else {
+                tmp->next = pl.value.strings;
+                pl.value.strings = tmp;
+            }
+            break;
+        }
+        case PTYPE_INTEGER: {
+            intList *tmp = g_new0(intList, 1);
+            tmp->value = pt->value.integer;
+            if (pl.value.integers == NULL) {
+                pl.value.integers = tmp;
+            } else {
+                tmp->next = pl.value.integers;
+                pl.value.integers = tmp;
+            }
+            break;
+        }
+        case PTYPE_NUMBER: {
+            numberList *tmp = g_new0(numberList, 1);
+            tmp->value = pt->value.number;
+            if (pl.value.numbers == NULL) {
+                pl.value.numbers = tmp;
+            } else {
+                tmp->next = pl.value.numbers;
+                pl.value.numbers = tmp;
+            }
+            break;
+        }
+        case PTYPE_BOOLEAN: {
+            boolList *tmp = g_new0(boolList, 1);
+            tmp->value = pt->value.boolean;
+            if (pl.value.booleans == NULL) {
+                pl.value.booleans = tmp;
+            } else {
+                tmp->next = pl.value.booleans;
+                pl.value.booleans = tmp;
+            }
+            break;
+        }
+        default:
+            g_assert(0);
+        }
+    }
+
+    ops->serialize((void **)&pl, &serialize_data, visit_primitive_list, &err);
+    ops->deserialize((void **)&pl_copy_ptr, serialize_data, visit_primitive_list, &err);
+
+    g_assert(err == NULL);
+    i = 0;
+
+    /* compare our deserialized list of primitives to the original */
+    do {
+        switch (pl_copy.type) {
+        case PTYPE_STRING: {
+            strList *ptr;
+            if (cur_head) {
+                ptr = cur_head;
+                cur_head = ptr->next;
+            } else {
+                cur_head = ptr = pl_copy.value.strings;
+            }
+            g_assert_cmpstr(pt->value.string, ==, ptr->value);
+            break;
+        }
+        case PTYPE_INTEGER: {
+            intList *ptr;
+            if (cur_head) {
+                ptr = cur_head;
+                cur_head = ptr->next;
+            } else {
+                cur_head = ptr = pl_copy.value.integers;
+            }
+            g_assert_cmpint(pt->value.integer, ==, ptr->value);
+            break;
+        }
+        case PTYPE_NUMBER: {
+            numberList *ptr;
+            GString *double_expected = g_string_new("");
+            GString *double_actual = g_string_new("");
+            if (cur_head) {
+                ptr = cur_head;
+                cur_head = ptr->next;
+            } else {
+                cur_head = ptr = pl_copy.value.numbers;
+            }
+            /* we serialize with %f for our reference visitors, so rather than
+             * fuzzy floating math to test "equality", just compare the
+             * formatted values
+             */
+            g_string_printf(double_expected, "%.6f", pt->value.number);
+            g_string_printf(double_actual, "%.6f", ptr->value);
+            g_assert_cmpstr(double_actual->str, ==, double_expected->str);
+            g_string_free(double_expected, true);
+            g_string_free(double_actual, true);
+            break;
+        }
+        case PTYPE_BOOLEAN: {
+            boolList *ptr;
+            if (cur_head) {
+                ptr = cur_head;
+                cur_head = ptr->next;
+            } else {
+                cur_head = ptr = pl_copy.value.booleans;
+            }
+            g_assert_cmpint(!!pt->value.boolean, ==, !!ptr->value);
+            break;
+        }
+        default:
+            g_assert(0);
+        }
+        i++;
+    } while (cur_head);
+
+    g_assert_cmpint(i, ==, 33);
+
+    ops->cleanup(serialize_data);
+    dealloc_helper(&pl, visit_primitive_list, &err);
+    g_assert(!err);
+    dealloc_helper(&pl_copy, visit_primitive_list, &err);
+    g_assert(!err);
+    g_free(args);
+}
+
 static void test_struct(gconstpointer opaque)
 {
     TestArgs *args = (TestArgs *) opaque;
@@ -719,7 +912,8 @@ static const SerializeOps visitors[] = {
         .serialize = qmp_serialize,
         .deserialize = qmp_deserialize,
         .cleanup = qmp_cleanup,
-        .caps = VCAP_PRIMITIVES | VCAP_STRUCTURES | VCAP_LISTS
+        .caps = VCAP_PRIMITIVES | VCAP_STRUCTURES | VCAP_LISTS |
+                VCAP_PRIMITIVE_LISTS
     },
     {
         .type = "String",
@@ -773,6 +967,24 @@ static void add_visitor_type(const SerializeOps *ops)
         args->test_data = NULL;
         g_test_add_data_func(testname, args, test_nested_struct_list);
     }
+
+    if (ops->caps & VCAP_PRIMITIVE_LISTS) {
+        i = 0;
+        while (pt_values[i].type != PTYPE_EOL) {
+            if (pt_values[i].type == PTYPE_STRING
+                || pt_values[i].type == PTYPE_NUMBER
+                || pt_values[i].type == PTYPE_BOOLEAN
+                || pt_values[i].type == PTYPE_INTEGER) {
+                sprintf(testname, "%s/primitive_list/%s", testname_prefix,
+                        pt_values[i].description);
+                args = g_malloc0(sizeof(*args));
+                args->ops = ops;
+                args->test_data = &pt_values[i];
+                g_test_add_data_func(testname, args, test_primitive_lists);
+            }
+            i++;
+        }
+    }
 }
 
 int main(int argc, char **argv)
-- 
1.7.9.5

[Qemu-devel] [PATCH 09/10] qapi: add native list coverage for QMP output visitor tests

[Michael Roth]

This exercises schema-generated visitors for native list types and does
some sanity checking on validity of serialized data.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi-schema-test.json           |    7 ++
 tests/test-qmp-output-visitor.c |  176 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 183 insertions(+)

diff --git a/qapi-schema-test.json b/qapi-schema-test.json
index 9eae350..d49cdb3 100644
--- a/qapi-schema-test.json
+++ b/qapi-schema-test.json
@@ -32,6 +32,13 @@
 { 'union': 'UserDefUnion',
   'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
 
+# for testing native lists
+{ 'union': 'UserDefNativeListUnion',
+  'data': { 'integer': ['int'],
+            'number': ['number'],
+            'boolean': ['bool'],
+            'string': ['str'] } }
+
 # testing commands
 { 'command': 'user_def_cmd', 'data': {} }
 { 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 71367e6..9bb8c71 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -431,6 +431,174 @@ static void test_visitor_out_union(TestOutputVisitorData *data,
     QDECREF(qdict);
 }
 
+static void init_native_list(UserDefNativeListUnion *cvalue)
+{
+    int i;
+    switch (cvalue->kind) {
+    case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER: {
+        intList **list = &cvalue->integer;
+        for (i = 0; i < 32; i++) {
+            *list = g_new0(intList, 1);
+            (*list)->value = i;
+            (*list)->next = NULL;
+            list = &(*list)->next;
+        }
+        break;
+    }
+    case USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN: {
+        boolList **list = &cvalue->boolean;
+        for (i = 0; i < 32; i++) {
+            *list = g_new0(boolList, 1);
+            (*list)->value = (i % 3 == 0);
+            (*list)->next = NULL;
+            list = &(*list)->next;
+        }
+        break;
+    }
+    case USER_DEF_NATIVE_LIST_UNION_KIND_STRING: {
+        strList **list = &cvalue->string;
+        for (i = 0; i < 32; i++) {
+            *list = g_new0(strList, 1);
+            (*list)->value = g_strdup_printf("%d", i);
+            (*list)->next = NULL;
+            list = &(*list)->next;
+        }
+        break;
+    }
+    case USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER: {
+        numberList **list = &cvalue->number;
+        for (i = 0; i < 32; i++) {
+            *list = g_new0(numberList, 1);
+            (*list)->value = (double)i / 3;
+            (*list)->next = NULL;
+            list = &(*list)->next;
+        }
+        break;
+    }
+    default:
+        g_assert(false);
+    }
+}
+
+static void check_native_list(QObject *qobj,
+                              UserDefNativeListUnionKind kind)
+{
+    QDict *qdict;
+    QList *qlist;
+    int i;
+
+    g_assert(qobj);
+    g_assert(qobject_type(qobj) == QTYPE_QDICT);
+    qdict = qobject_to_qdict(qobj);
+    g_assert(qdict);
+    g_assert(qdict_haskey(qdict, "data"));
+    qlist = qlist_copy(qobject_to_qlist(qdict_get(qdict, "data")));
+
+    switch (kind) {
+    case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER:
+        for (i = 0; i < 32; i++) {
+            QObject *tmp;
+            QInt *qvalue;
+            tmp = qlist_peek(qlist);
+            g_assert(tmp);
+            qvalue = qobject_to_qint(tmp);
+            g_assert_cmpint(qint_get_int(qvalue), ==, i);
+            qobject_decref(qlist_pop(qlist));
+        }
+        break;
+    case USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN:
+        for (i = 0; i < 32; i++) {
+            QObject *tmp;
+            QBool *qvalue;
+            tmp = qlist_peek(qlist);
+            g_assert(tmp);
+            qvalue = qobject_to_qbool(tmp);
+            g_assert_cmpint(qbool_get_int(qvalue), ==, (i % 3 == 0) ? 1 : 0);
+            qobject_decref(qlist_pop(qlist));
+        }
+        break;
+    case USER_DEF_NATIVE_LIST_UNION_KIND_STRING:
+        for (i = 0; i < 32; i++) {
+            QObject *tmp;
+            QString *qvalue;
+            gchar str[8];
+            tmp = qlist_peek(qlist);
+            g_assert(tmp);
+            qvalue = qobject_to_qstring(tmp);
+            sprintf(str, "%d", i);
+            g_assert_cmpstr(qstring_get_str(qvalue), ==, str);
+            qobject_decref(qlist_pop(qlist));
+        }
+        break;
+    case USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER:
+        for (i = 0; i < 32; i++) {
+            QObject *tmp;
+            QFloat *qvalue;
+            GString *double_expected = g_string_new("");
+            GString *double_actual = g_string_new("");
+
+            tmp = qlist_peek(qlist);
+            g_assert(tmp);
+            qvalue = qobject_to_qfloat(tmp);
+            g_string_printf(double_expected, "%.6f", (double)i / 3);
+            g_string_printf(double_actual, "%.6f", qfloat_get_double(qvalue));
+            g_assert_cmpstr(double_actual->str, ==, double_expected->str);
+
+            qobject_decref(qlist_pop(qlist));
+            g_string_free(double_expected, true);
+            g_string_free(double_actual, true);
+        }
+        break;
+    default:
+        g_assert(false);
+    }
+    QDECREF(qlist);
+}
+
+static void test_native_list(TestOutputVisitorData *data,
+                             const void *unused,
+                             UserDefNativeListUnionKind kind)
+{
+    UserDefNativeListUnion *cvalue = g_new0(UserDefNativeListUnion, 1);
+    Error *err = NULL;
+    QObject *obj;
+
+    cvalue->kind = kind;
+    init_native_list(cvalue);
+
+    visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &err);
+    g_assert(err == NULL);
+
+    obj = qmp_output_get_qobject(data->qov);
+    check_native_list(obj, cvalue->kind);
+    qapi_free_UserDefNativeListUnion(cvalue);
+    qobject_decref(obj);
+}
+
+static void test_visitor_out_native_list_int(TestOutputVisitorData *data,
+                                             const void *unused)
+{
+    test_native_list(data, unused, USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER);
+}
+
+static void test_visitor_out_native_list_bool(TestOutputVisitorData *data,
+                                              const void *unused)
+{
+    test_native_list(data, unused, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
+}
+
+static void test_visitor_out_native_list_str(TestOutputVisitorData *data,
+                                              const void *unused)
+{
+    test_native_list(data, unused, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
+}
+
+static void test_visitor_out_native_list_number(TestOutputVisitorData *data,
+                                                const void *unused)
+{
+    test_native_list(data, unused, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
+}
+
 static void output_visitor_test_add(const char *testpath,
                                     TestOutputVisitorData *data,
                                     void (*test_func)(TestOutputVisitorData *data, const void *user_data))
@@ -471,6 +639,14 @@ int main(int argc, char **argv)
                             &out_visitor_data, test_visitor_out_list_qapi_free);
     output_visitor_test_add("/visitor/output/union",
                             &out_visitor_data, test_visitor_out_union);
+    output_visitor_test_add("/visitor/output/native_list/int",
+                            &out_visitor_data, test_visitor_out_native_list_int);
+    output_visitor_test_add("/visitor/output/native_list/bool",
+                            &out_visitor_data, test_visitor_out_native_list_bool);
+    output_visitor_test_add("/visitor/output/native_list/string",
+                            &out_visitor_data, test_visitor_out_native_list_str);
+    output_visitor_test_add("/visitor/output/native_list/number",
+                            &out_visitor_data, test_visitor_out_native_list_number);
 
     g_test_run();
 
-- 
1.7.9.5

[Qemu-devel] [PATCH 10/10] qapi: add native list coverage for QMP input visitor tests

[Michael Roth]

This exercises schema-generated visitors for native list types and does
some sanity checking on validity of deserialized data.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/test-qmp-input-visitor.c |  186 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 186 insertions(+)

diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 955a4c0..483a20b 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -61,6 +61,31 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
     return v;
 }
 
+/* similar to visitor_input_test_init(), but does not expect a string
+ * literal/format json_string argument and so can be used for
+ * programatically generated strings (and we can't pass in programatically
+ * generated strings via %s format parameters since qobject_from_jsonv()
+ * will wrap those in double-quotes and treat the entire object as a
+ * string)
+ */
+static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
+                                            const char *json_string)
+{
+    Visitor *v;
+
+    data->obj = qobject_from_json(json_string);
+
+    g_assert(data->obj != NULL);
+
+    data->qiv = qmp_input_visitor_new(data->obj);
+    g_assert(data->qiv != NULL);
+
+    v = qmp_input_get_visitor(data->qiv);
+    g_assert(v != NULL);
+
+    return v;
+}
+
 static void test_visitor_in_int(TestInputVisitorData *data,
                                 const void *unused)
 {
@@ -259,6 +284,159 @@ static void test_visitor_in_union(TestInputVisitorData *data,
     qapi_free_UserDefUnion(tmp);
 }
 
+static void test_visitor_in_native_list_int(TestInputVisitorData *data,
+                                            const void *unused)
+{
+    UserDefNativeListUnion *cvalue = NULL;
+    intList *elem = NULL;
+    Error *err = NULL;
+    Visitor *v;
+    GString *gstr_list = g_string_new("");
+    GString *gstr_union = g_string_new("");
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        g_string_append_printf(gstr_list, "%d", i);
+        if (i != 31) {
+            g_string_append(gstr_list, ", ");
+        }
+    }
+    g_string_append_printf(gstr_union,  "{ 'type': 'integer', 'data': [ %s ] }",
+                           gstr_list->str);
+    v = visitor_input_test_init_raw(data,  gstr_union->str);
+
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
+    g_assert(err == NULL);
+    g_assert(cvalue != NULL);
+    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER);
+
+    for (i = 0, elem = cvalue->integer; elem; elem = elem->next, i++) {
+        g_assert_cmpint(elem->value, ==, i);
+    }
+
+    g_string_free(gstr_union, true);
+    g_string_free(gstr_list, true);
+    qapi_free_UserDefNativeListUnion(cvalue);
+}
+
+static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
+                                            const void *unused)
+{
+    UserDefNativeListUnion *cvalue = NULL;
+    boolList *elem = NULL;
+    Error *err = NULL;
+    Visitor *v;
+    GString *gstr_list = g_string_new("");
+    GString *gstr_union = g_string_new("");
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        g_string_append_printf(gstr_list, "%s",
+                               (i % 3 == 0) ? "true" : "false");
+        if (i != 31) {
+            g_string_append(gstr_list, ", ");
+        }
+    }
+    g_string_append_printf(gstr_union,  "{ 'type': 'boolean', 'data': [ %s ] }",
+                           gstr_list->str);
+    v = visitor_input_test_init_raw(data,  gstr_union->str);
+
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
+    g_assert(err == NULL);
+    g_assert(cvalue != NULL);
+    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
+
+    for (i = 0, elem = cvalue->boolean; elem; elem = elem->next, i++) {
+        g_assert_cmpint(elem->value, ==, (i % 3 == 0) ? 1 : 0);
+    }
+
+    g_string_free(gstr_union, true);
+    g_string_free(gstr_list, true);
+    qapi_free_UserDefNativeListUnion(cvalue);
+}
+
+static void test_visitor_in_native_list_string(TestInputVisitorData *data,
+                                               const void *unused)
+{
+    UserDefNativeListUnion *cvalue = NULL;
+    strList *elem = NULL;
+    Error *err = NULL;
+    Visitor *v;
+    GString *gstr_list = g_string_new("");
+    GString *gstr_union = g_string_new("");
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        g_string_append_printf(gstr_list, "'%d'", i);
+        if (i != 31) {
+            g_string_append(gstr_list, ", ");
+        }
+    }
+    g_string_append_printf(gstr_union,  "{ 'type': 'string', 'data': [ %s ] }",
+                           gstr_list->str);
+    v = visitor_input_test_init_raw(data,  gstr_union->str);
+
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
+    g_assert(err == NULL);
+    g_assert(cvalue != NULL);
+    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
+
+    for (i = 0, elem = cvalue->string; elem; elem = elem->next, i++) {
+        gchar str[8];
+        sprintf(str, "%d", i);
+        g_assert_cmpstr(elem->value, ==, str);
+    }
+
+    g_string_free(gstr_union, true);
+    g_string_free(gstr_list, true);
+    qapi_free_UserDefNativeListUnion(cvalue);
+}
+
+#define DOUBLE_STR_MAX 16
+
+static void test_visitor_in_native_list_number(TestInputVisitorData *data,
+                                               const void *unused)
+{
+    UserDefNativeListUnion *cvalue = NULL;
+    numberList *elem = NULL;
+    Error *err = NULL;
+    Visitor *v;
+    GString *gstr_list = g_string_new("");
+    GString *gstr_union = g_string_new("");
+    int i;
+
+    for (i = 0; i < 32; i++) {
+        g_string_append_printf(gstr_list, "%f", (double)i / 3);
+        if (i != 31) {
+            g_string_append(gstr_list, ", ");
+        }
+    }
+    g_string_append_printf(gstr_union,  "{ 'type': 'number', 'data': [ %s ] }",
+                           gstr_list->str);
+    v = visitor_input_test_init_raw(data,  gstr_union->str);
+
+    visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
+    g_assert(err == NULL);
+    g_assert(cvalue != NULL);
+    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
+
+    for (i = 0, elem = cvalue->number; elem; elem = elem->next, i++) {
+        GString *double_expected = g_string_new("");
+        GString *double_actual = g_string_new("");
+
+        g_string_printf(double_expected, "%.6f", (double)i / 3);
+        g_string_printf(double_actual, "%.6f", elem->value);
+        g_assert_cmpstr(double_expected->str, ==, double_actual->str);
+
+        g_string_free(double_expected, true);
+        g_string_free(double_actual, true);
+    }
+
+    g_string_free(gstr_union, true);
+    g_string_free(gstr_list, true);
+    qapi_free_UserDefNativeListUnion(cvalue);
+}
+
 static void input_visitor_test_add(const char *testpath,
                                    TestInputVisitorData *data,
                                    void (*test_func)(TestInputVisitorData *data, const void *user_data))
@@ -310,6 +488,14 @@ int main(int argc, char **argv)
                             &in_visitor_data, test_visitor_in_union);
     input_visitor_test_add("/visitor/input/errors",
                             &in_visitor_data, test_visitor_in_errors);
+    input_visitor_test_add("/visitor/input/native_list/int",
+                            &in_visitor_data, test_visitor_in_native_list_int);
+    input_visitor_test_add("/visitor/input/native_list/bool",
+                            &in_visitor_data, test_visitor_in_native_list_bool);
+    input_visitor_test_add("/visitor/input/native_list/str",
+                            &in_visitor_data, test_visitor_in_native_list_string);
+    input_visitor_test_add("/visitor/input/native_list/number",
+                            &in_visitor_data, test_visitor_in_native_list_number);
 
     g_test_run();
 
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v2 00/10] qapi: add support for lists of native types

[Luiz Capitulino]

On Thu,  9 May 2013 21:20:52 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> These patches apply on top of qemu.git master, and can also be obtained from:
> git://github.com/mdroth/qemu.git qapi-native-lists
> 
> Sending this now since a number of series have popped up in the past that
> wanted this, and Amos has some pending patches (query-mac-tables) that rely
> on this as well.
> 
> These patches add support for specifying lists of native qapi types
> (int/bool/str/number) like so:
> 
>   { 'type': 'foo',
>     'data': { 'bar': ['int'] }}
> 
> for a 'bar' field that is a list of type 'int',
> 
>   { 'type': 'foo2',
>     'data': { 'bar2': ['str'] }}
> 
> for a 'bar2' field that is a list of type 'str', and so on.
> 
> This uses linked list types for the native C representations, just as we do
> for complex schema-defined types. In the future we may add schema annotations
> of some sort to specify a more natural/efficient array type for the C
> representations, but this should serve the majority of uses-cases for now.

Series looks good to me. I'd drop patch 06/10 if it's not required
for this series though.

Re: [Qemu-devel] [PATCH v2 00/10] qapi: add support for lists of native types

[Laszlo Ersek]

On 05/10/13 17:30, Luiz Capitulino wrote:
> On Thu,  9 May 2013 21:20:52 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
>> These patches apply on top of qemu.git master, and can also be obtained from:
>> git://github.com/mdroth/qemu.git qapi-native-lists
>>
>> Sending this now since a number of series have popped up in the past that
>> wanted this, and Amos has some pending patches (query-mac-tables) that rely
>> on this as well.
>>
>> These patches add support for specifying lists of native qapi types
>> (int/bool/str/number) like so:
>>
>>   { 'type': 'foo',
>>     'data': { 'bar': ['int'] }}
>>
>> for a 'bar' field that is a list of type 'int',
>>
>>   { 'type': 'foo2',
>>     'data': { 'bar2': ['str'] }}
>>
>> for a 'bar2' field that is a list of type 'str', and so on.
>>
>> This uses linked list types for the native C representations, just as we do
>> for complex schema-defined types. In the future we may add schema annotations
>> of some sort to specify a more natural/efficient array type for the C
>> representations, but this should serve the majority of uses-cases for now.
> 
> Series looks good to me. I'd drop patch 06/10 if it's not required
> for this series though.

I guess:

On 05/10/13 04:20, Michael Roth wrote:
> v1->v2:
>  * fixed do-nothing float tests in pre-existing code and updated new
>    unit tests accordingly (Laszlo)
>  * added a fix for a bug in json parser that was exposed by above change

Laszlo

Re: [Qemu-devel] [PATCH v2 00/10] qapi: add support for lists of native types

[Luiz Capitulino]

On Fri, 10 May 2013 17:40:20 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> On 05/10/13 17:30, Luiz Capitulino wrote:
> > On Thu,  9 May 2013 21:20:52 -0500
> > Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> > 
> >> These patches apply on top of qemu.git master, and can also be obtained from:
> >> git://github.com/mdroth/qemu.git qapi-native-lists
> >>
> >> Sending this now since a number of series have popped up in the past that
> >> wanted this, and Amos has some pending patches (query-mac-tables) that rely
> >> on this as well.
> >>
> >> These patches add support for specifying lists of native qapi types
> >> (int/bool/str/number) like so:
> >>
> >>   { 'type': 'foo',
> >>     'data': { 'bar': ['int'] }}
> >>
> >> for a 'bar' field that is a list of type 'int',
> >>
> >>   { 'type': 'foo2',
> >>     'data': { 'bar2': ['str'] }}
> >>
> >> for a 'bar2' field that is a list of type 'str', and so on.
> >>
> >> This uses linked list types for the native C representations, just as we do
> >> for complex schema-defined types. In the future we may add schema annotations
> >> of some sort to specify a more natural/efficient array type for the C
> >> representations, but this should serve the majority of uses-cases for now.
> > 
> > Series looks good to me. I'd drop patch 06/10 if it's not required
> > for this series though.
> 
> I guess:

I see, although it's still a separate issue (which I'd personally fix
in a different series).

> 
> On 05/10/13 04:20, Michael Roth wrote:
> > v1->v2:
> >  * fixed do-nothing float tests in pre-existing code and updated new
> >    unit tests accordingly (Laszlo)
> >  * added a fix for a bug in json parser that was exposed by above change
> 
> Laszlo
>