[Qemu-devel] [PATCH v4 00/16] Fix qapi mangling of downstream names

[Qemu-devel] [PATCH v4 00/16] Fix qapi mangling of downstream names

[Eric Blake]

This series makes it possible to use downstream extensions
(such as __com.redhat_xyz) and temporary names (such as x-foo)
in every position possible in QAPI schemes, with added tests
that the generated code still compiles.

There's still some things we could do to the qapi generator,
such as normalizing struct member names and C manglings and
creating named implicit types up front on the initial parse
rather than multiple times in each backend.  But that should
wait until existing pending patches have landed, to minimize
rebase churn.

v3 was here:
https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00519.html
[already queued in Markus tree, but not yet in a pull request, so
that queue can be ruthlessly rebased]

v4 includes 2 new patches (although 14/14 is somewhat unrelated,
and could easily be dropped from the series if it needs another
spin without holding up the rest of the series), incorporates
Markus' suggestions for more comments, and simplifies special-casing
of lists of builtin types.

Here's the backport-diff stats (and note that it exposes a bug
in the script, as a raw % in the subject line triggered mayhem):

001/16:[----] [--] 'qapi: Fix C identifiers generated for names containing '.''
002/16:[----] [--] 'qapi: Rename identical c_fun()/c_var() into c_name()'
003/16:[----] [--] 'qapi: Rename _generate_enum_string() to camel_to_upper()'
004/16:[----] [--] 'qapi: Rename generate_enum_full_value() to c_enum_const()'
005/16:[----] [--] 'qapi: Simplify c_enum_const()'
006/16:[----] [--] 'qapi: Use c_enum_const() in generate_alternate_qtypes()'
007/16:[----] [--] 'qapi: Move camel_to_upper(), c_enum_const() to closely related code'
008/16:[down] 'qapi: Tidy c_type logic'
009/16:[0046] [FC] 'qapi: Make c_type() consistently convert qapi names'
010/16:[0007] [FC] 'qapi: Support downstream enums'
011/16:[----] [--] 'qapi: Support downstream structs'
012/16:[----] [--] 'qapi: Support downstream simple unions'
013/16:[----] [--] 'qapi: Support downstream flat unions'
014/16:[----] [--] 'qapi: Support downstream alternates'
015/16:[----] [--] 'qapi: Support downstream events and commands'
/home/eblake/bin/git-backport-diff: line 267: printf: `v': invalid format character
016/16:[down] 'qapi: Prefer '"str" + var' over '"str"

Eric Blake (10):
  qapi: Rename identical c_fun()/c_var() into c_name()
  qapi: Tidy c_type logic
  qapi: Make c_type() consistently convert qapi names
  qapi: Support downstream enums
  qapi: Support downstream structs
  qapi: Support downstream simple unions
  qapi: Support downstream flat unions
  qapi: Support downstream alternates
  qapi: Support downstream events and commands
  qapi: Prefer '"str" + var' over '"str%s" % var'

Markus Armbruster (6):
  qapi: Fix C identifiers generated for names containing '.'
  qapi: Rename _generate_enum_string() to camel_to_upper()
  qapi: Rename generate_enum_full_value() to c_enum_const()
  qapi: Simplify c_enum_const()
  qapi: Use c_enum_const() in generate_alternate_qtypes()
  qapi: Move camel_to_upper(), c_enum_const() to closely related code

 scripts/qapi-commands.py                |  56 ++++----
 scripts/qapi-event.py                   |  15 +--
 scripts/qapi-types.py                   |  54 ++++----
 scripts/qapi-visit.py                   |  55 ++++----
 scripts/qapi.py                         | 226 +++++++++++++++++---------------
 tests/qapi-schema/include-non-file.err  |   2 +-
 tests/qapi-schema/qapi-schema-test.json |  20 +++
 tests/qapi-schema/qapi-schema-test.out  |  21 ++-
 tests/test-qmp-commands.c               |  15 +++
 9 files changed, 263 insertions(+), 201 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v4 01/16] qapi: Fix C identifiers generated for names containing '.'

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

c_fun() maps '.' to '_', c_var() doesn't.  Nothing prevents '.' in
QAPI names that get passed to c_var().

Which QAPI names get passed to c_fun(), to c_var(), or to both is not
obvious.  Names of command parameters and struct type members get
passed to c_var().

c_var() strips a leading '*', but this cannot happen.  c_fun()
doesn't.

Fix c_var() to work exactly like c_fun().

Perhaps they should be replaced by a single mapping function.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
[add 'import string']
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 scripts/qapi.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 166b74f..4b07805 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -15,6 +15,7 @@ import re
 from ordereddict import OrderedDict
 import os
 import sys
+import string

 builtin_types = {
     'str':      'QTYPE_QSTRING',
@@ -752,6 +753,8 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name

+c_var_trans = string.maketrans('.-', '__')
+
 def c_var(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
@@ -781,10 +784,10 @@ def c_var(name, protect=True):
     polluted_words = set(['unix', 'errno'])
     if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words):
         return "q_" + name
-    return name.replace('-', '_').lstrip("*")
+    return name.translate(c_var_trans)

 def c_fun(name, protect=True):
-    return c_var(name, protect).replace('.', '_')
+    return c_var(name, protect)

 def c_list_type(name):
     return '%sList' % name
-- 
2.1.0

[Qemu-devel] [PATCH v4 02/16] qapi: Rename identical c_fun()/c_var() into c_name()

[Eric Blake]

Now that the two functions are identical, we only need one of them,
and we might as well give it a more descriptive name.  Basically,
the function serves as the translation from a QAPI name into a
(portion of a) C identifier, without regards to whether it is a
variable or function name.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-commands.py | 35 ++++++++++++++++++-----------------
 scripts/qapi-event.py    | 10 +++++-----
 scripts/qapi-types.py    |  8 ++++----
 scripts/qapi-visit.py    | 10 +++++-----
 scripts/qapi.py          | 11 ++++-------
 5 files changed, 36 insertions(+), 38 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 93e43f0..8c125ca 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -31,12 +31,13 @@ def generate_command_decl(name, args, ret_type):
     for argname, argtype, optional in parse_args(args):
         argtype = c_type(argtype, is_param=True)
         if optional:
-            arglist += "bool has_%s, " % c_var(argname)
-        arglist += "%s %s, " % (argtype, c_var(argname))
+            arglist += "bool has_%s, " % c_name(argname)
+        arglist += "%s %s, " % (argtype, c_name(argname))
     return mcgen('''
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
 ''',
-                 ret_type=c_type(ret_type), name=c_fun(name), args=arglist).strip()
+                 ret_type=c_type(ret_type), name=c_name(name),
+                 args=arglist).strip()

 def gen_err_check(errvar):
     if errvar:
@@ -55,14 +56,14 @@ def gen_sync_call(name, args, ret_type, indent=0):
         retval = "retval = "
     for argname, argtype, optional in parse_args(args):
         if optional:
-            arglist += "has_%s, " % c_var(argname)
-        arglist += "%s, " % (c_var(argname))
+            arglist += "has_%s, " % c_name(argname)
+        arglist += "%s, " % (c_name(argname))
     push_indent(indent)
     ret = mcgen('''
 %(retval)sqmp_%(name)s(%(args)s&local_err);

 ''',
-                name=c_fun(name), args=arglist, retval=retval).rstrip()
+                name=c_name(name), args=arglist, retval=retval).rstrip()
     if ret_type:
         ret += "\n" + gen_err_check('local_err')
         ret += "\n" + mcgen(''''
@@ -76,7 +77,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
 def gen_marshal_output_call(name, ret_type):
     if not ret_type:
         return ""
-    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_fun(name)
+    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)

 def gen_visitor_input_containers_decl(args, obj):
     ret = ""
@@ -101,17 +102,17 @@ def gen_visitor_input_vars_decl(args):
             ret += mcgen('''
 bool has_%(argname)s = false;
 ''',
-                         argname=c_var(argname))
+                         argname=c_name(argname))
         if is_c_ptr(argtype):
             ret += mcgen('''
 %(argtype)s %(argname)s = NULL;
 ''',
-                         argname=c_var(argname), argtype=c_type(argtype))
+                         argname=c_name(argname), argtype=c_type(argtype))
         else:
             ret += mcgen('''
 %(argtype)s %(argname)s = {0};
 ''',
-                         argname=c_var(argname), argtype=c_type(argtype))
+                         argname=c_name(argname), argtype=c_type(argtype))

     pop_indent()
     return ret.rstrip()
@@ -144,17 +145,17 @@ v = qmp_input_get_visitor(mi);
             ret += mcgen('''
 visit_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s);
 ''',
-                         c_name=c_var(argname), name=argname, errp=errparg)
+                         c_name=c_name(argname), name=argname, errp=errparg)
             ret += gen_err_check(errarg)
             ret += mcgen('''
 if (has_%(c_name)s) {
 ''',
-                         c_name=c_var(argname))
+                         c_name=c_name(argname))
             push_indent()
         ret += mcgen('''
 %(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
-                     c_name=c_var(argname), name=argname, argtype=argtype,
+                     c_name=c_name(argname), name=argname, argtype=argtype,
                      visitor=type_visitor(argtype), errp=errparg)
         ret += gen_err_check(errarg)
         if optional:
@@ -198,16 +199,16 @@ out:
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-                c_ret_type=c_type(ret_type), c_name=c_fun(name),
+                c_ret_type=c_type(ret_type), c_name=c_name(name),
                 visitor=type_visitor(ret_type))

     return ret

 def gen_marshal_input_decl(name, args, ret_type, middle_mode):
     if middle_mode:
-        return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict, QObject **ret)' % c_fun(name)
+        return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict, QObject **ret)' % c_name(name)
     else:
-        return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_fun(name)
+        return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)



@@ -304,7 +305,7 @@ def gen_registry(commands):
         registry += mcgen('''
 qmp_register_command("%(name)s", qmp_marshal_input_%(c_name)s, %(opts)s);
 ''',
-                     name=cmd['command'], c_name=c_fun(cmd['command']),
+                     name=cmd['command'], c_name=c_name(cmd['command']),
                      opts=options)
     pop_indent()
     ret = mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index 47dc041..c4612e3 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -17,17 +17,17 @@ import getopt
 import errno

 def _generate_event_api_name(event_name, params):
-    api_name = "void qapi_event_send_%s(" % c_fun(event_name).lower();
+    api_name = "void qapi_event_send_%s(" % c_name(event_name).lower();
     l = len(api_name)

     if params:
         for argname, argentry, optional in parse_args(params):
             if optional:
-                api_name += "bool has_%s,\n" % c_var(argname)
+                api_name += "bool has_%s,\n" % c_name(argname)
                 api_name += "".ljust(l)

             api_name += "%s %s,\n" % (c_type(argentry, is_param=True),
-                                      c_var(argname))
+                                      c_name(argname))
             api_name += "".ljust(l)

     api_name += "Error **errp)"
@@ -98,7 +98,7 @@ def generate_event_implement(api_name, event_name, params):
                 ret += mcgen("""
     if (has_%(var)s) {
 """,
-                             var = c_var(argname))
+                             var = c_name(argname))
                 push_indent()

             if argentry == "str":
@@ -113,7 +113,7 @@ def generate_event_implement(api_name, event_name, params):
     }
 """,
                          var_type = var_type,
-                         var = c_var(argname),
+                         var = c_name(argname),
                          type = type_name(argentry),
                          name = argname)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 2bf8145..e74cabe 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -68,11 +68,11 @@ def generate_struct_fields(members):
             ret += mcgen('''
     bool has_%(c_name)s;
 ''',
-                         c_name=c_var(argname))
+                         c_name=c_name(argname))
         ret += mcgen('''
     %(c_type)s %(c_name)s;
 ''',
-                     c_type=c_type(argentry), c_name=c_var(argname))
+                     c_type=c_type(argentry), c_name=c_name(argname))

     return ret

@@ -184,7 +184,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
 ''',
         qtype = qtype,
         abbrev = de_camel_case(name).upper(),
-        enum = c_fun(de_camel_case(key),False).upper())
+        enum = c_name(de_camel_case(key),False).upper())

     ret += mcgen('''
 };
@@ -221,7 +221,7 @@ struct %(name)s
         %(c_type)s %(c_name)s;
 ''',
                      c_type=c_type(typeinfo[key]),
-                     c_name=c_fun(key))
+                     c_name=c_name(key))

     ret += mcgen('''
     };
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0e67b33..ba623b1 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -66,7 +66,7 @@ if (err) {
     goto out;
 }
 ''',
-                     type=type_name(base), c_name=c_var('base'))
+                     type=type_name(base), c_name=c_name('base'))

     for argname, argentry, optional in parse_args(members):
         if optional:
@@ -74,13 +74,13 @@ if (err) {
 visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
 if (!err && (*obj)->has_%(c_name)s) {
 ''',
-                         c_name=c_var(argname), name=argname)
+                         c_name=c_name(argname), name=argname)
             push_indent()

         ret += mcgen('''
 visit_type_%(type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
 ''',
-                     type=type_name(argentry), c_name=c_var(argname),
+                     type=type_name(argentry), c_name=c_name(argname),
                      name=argname)

         if optional:
@@ -222,7 +222,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 ''',
                 enum_full_value = enum_full_value,
                 c_type = type_name(members[key]),
-                c_name = c_fun(key))
+                c_name = c_name(key))

     ret += mcgen('''
     default:
@@ -323,7 +323,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 ''',
                 enum_full_value = enum_full_value,
                 c_type=type_name(members[key]),
-                c_name=c_fun(key))
+                c_name=c_name(key))

     ret += mcgen('''
         default:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 4b07805..a4701ca 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -753,9 +753,9 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name

-c_var_trans = string.maketrans('.-', '__')
+c_name_trans = string.maketrans('.-', '__')

-def c_var(name, protect=True):
+def c_name(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
                      'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
@@ -784,10 +784,7 @@ def c_var(name, protect=True):
     polluted_words = set(['unix', 'errno'])
     if protect and (name in c89_words | c99_words | c11_words | gcc_words | cpp_words | polluted_words):
         return "q_" + name
-    return name.translate(c_var_trans)
-
-def c_fun(name, protect=True):
-    return c_var(name, protect)
+    return name.translate(c_name_trans)

 def c_list_type(name):
     return '%sList' % name
@@ -945,7 +942,7 @@ def guardend(name):
 # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
 # ENUM24_Name -> ENUM24_NAME
 def _generate_enum_string(value):
-    c_fun_str = c_fun(value, False)
+    c_fun_str = c_name(value, False)
     if value.isupper():
         return c_fun_str

-- 
2.1.0

[Qemu-devel] [PATCH v4 03/16] qapi: Rename _generate_enum_string() to camel_to_upper()

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index a4701ca..7330f7c 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -538,7 +538,7 @@ def check_union(expr, expr_info):

         # Otherwise, check for conflicts in the generated enum
         else:
-            c_key = _generate_enum_string(key)
+            c_key = camel_to_upper(key)
             if c_key in values:
                 raise QAPIExprError(expr_info,
                                     "Union '%s' member '%s' clashes with '%s'"
@@ -556,7 +556,7 @@ def check_alternate(expr, expr_info):
         check_name(expr_info, "Member of alternate '%s'" % name, key)

         # Check for conflicts in the generated enum
-        c_key = _generate_enum_string(key)
+        c_key = camel_to_upper(key)
         if c_key in values:
             raise QAPIExprError(expr_info,
                                 "Alternate '%s' member '%s' clashes with '%s'"
@@ -587,7 +587,7 @@ def check_enum(expr, expr_info):
     for member in members:
         check_name(expr_info, "Member of enum '%s'" %name, member,
                    enum_member=True)
-        key = _generate_enum_string(member)
+        key = camel_to_upper(member)
         if key in values:
             raise QAPIExprError(expr_info,
                                 "Enum '%s' member '%s' clashes with '%s'"
@@ -941,7 +941,7 @@ def guardend(name):
 # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
 # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
 # ENUM24_Name -> ENUM24_NAME
-def _generate_enum_string(value):
+def camel_to_upper(value):
     c_fun_str = c_name(value, False)
     if value.isupper():
         return c_fun_str
@@ -961,6 +961,6 @@ def _generate_enum_string(value):
     return new_name.lstrip('_').upper()

 def generate_enum_full_value(enum_name, enum_value):
-    abbrev_string = _generate_enum_string(enum_name)
-    value_string = _generate_enum_string(enum_value)
+    abbrev_string = camel_to_upper(enum_name)
+    value_string = camel_to_upper(enum_value)
     return "%s_%s" % (abbrev_string, value_string)
-- 
2.1.0

[Qemu-devel] [PATCH v4 04/16] qapi: Rename generate_enum_full_value() to c_enum_const()

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-event.py | 5 ++---
 scripts/qapi-types.py | 6 +++---
 scripts/qapi-visit.py | 4 ++--
 scripts/qapi.py       | 6 +++---
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index c4612e3..a7e0033 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -177,7 +177,7 @@ typedef enum %(event_enum_name)s
                       event_enum_name = event_enum_name)

     # append automatically generated _MAX value
-    enum_max_value = generate_enum_full_value(event_enum_name, "MAX")
+    enum_max_value = c_enum_const(event_enum_name, "MAX")
     enum_values = event_enum_values + [ enum_max_value ]

     i = 0
@@ -343,8 +343,7 @@ for expr in exprs:
         fdecl.write(ret)

         # We need an enum value per event
-        event_enum_value = generate_enum_full_value(event_enum_name,
-                                                    event_name)
+        event_enum_value = c_enum_const(event_enum_name, event_name)
         ret = generate_event_implement(api_name, event_name, params)
         fdef.write(ret)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index e74cabe..6ca48c1 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -118,13 +118,13 @@ const char *%(name)s_lookup[] = {
                          name=name)
     i = 0
     for value in values:
-        index = generate_enum_full_value(name, value)
+        index = c_enum_const(name, value)
         ret += mcgen('''
     [%(index)s] = "%(value)s",
 ''',
                      index = index, value = value)

-    max_index = generate_enum_full_value(name, 'MAX')
+    max_index = c_enum_const(name, 'MAX')
     ret += mcgen('''
     [%(max_index)s] = NULL,
 };
@@ -150,7 +150,7 @@ typedef enum %(name)s

     i = 0
     for value in enum_values:
-        enum_full_value = generate_enum_full_value(name, value)
+        enum_full_value = c_enum_const(name, value)
         enum_decl += mcgen('''
     %(enum_full_value)s = %(i)d,
 ''',
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index ba623b1..0368e62 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -214,7 +214,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
             or find_union(members[key])
             or find_enum(members[key])), "Invalid alternate member"

-        enum_full_value = generate_enum_full_value(disc_type, key)
+        enum_full_value = c_enum_const(disc_type, key)
         ret += mcgen('''
     case %(enum_full_value)s:
         visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
@@ -315,7 +315,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
         else:
             fmt = 'visit_type_implicit_%(c_type)s(m, &(*obj)->%(c_name)s, &err);'

-        enum_full_value = generate_enum_full_value(disc_type, key)
+        enum_full_value = c_enum_const(disc_type, key)
         ret += mcgen('''
         case %(enum_full_value)s:
             ''' + fmt + '''
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7330f7c..1258f76 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -960,7 +960,7 @@ def camel_to_upper(value):
         new_name += c
     return new_name.lstrip('_').upper()

-def generate_enum_full_value(enum_name, enum_value):
-    abbrev_string = camel_to_upper(enum_name)
-    value_string = camel_to_upper(enum_value)
+def c_enum_const(type_name, const_name):
+    abbrev_string = camel_to_upper(type_name)
+    value_string = camel_to_upper(const_name)
     return "%s_%s" % (abbrev_string, value_string)
-- 
2.1.0

[Qemu-devel] [PATCH v4 05/16] qapi: Simplify c_enum_const()

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1258f76..b917cad 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -961,6 +961,4 @@ def camel_to_upper(value):
     return new_name.lstrip('_').upper()

 def c_enum_const(type_name, const_name):
-    abbrev_string = camel_to_upper(type_name)
-    value_string = camel_to_upper(const_name)
-    return "%s_%s" % (abbrev_string, value_string)
+    return camel_to_upper(type_name + '_' + const_name)
-- 
2.1.0

[Qemu-devel] [PATCH v4 06/16] qapi: Use c_enum_const() in generate_alternate_qtypes()

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

Missed in commit b0b5819.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-types.py |  6 ++----
 scripts/qapi.py       | 11 -----------
 2 files changed, 2 insertions(+), 15 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 6ca48c1..9eb08a6 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -180,11 +180,9 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
         assert qtype, "Invalid alternate member"

         ret += mcgen('''
-    [ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s,
+    [ %(qtype)s ] = %(enum_const)s,
 ''',
-        qtype = qtype,
-        abbrev = de_camel_case(name).upper(),
-        enum = c_name(de_camel_case(key),False).upper())
+        qtype = qtype, enum_const = c_enum_const(name + 'Kind', key))

     ret += mcgen('''
 };
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b917cad..3757a91 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -729,17 +729,6 @@ def parse_args(typeinfo):
         # value of an optional argument.
         yield (argname, argentry, optional)

-def de_camel_case(name):
-    new_name = ''
-    for ch in name:
-        if ch.isupper() and new_name:
-            new_name += '_'
-        if ch == '-':
-            new_name += '_'
-        else:
-            new_name += ch.lower()
-    return new_name
-
 def camel_case(name):
     new_name = ''
     first = True
-- 
2.1.0

[Qemu-devel] [PATCH v4 07/16] qapi: Move camel_to_upper(), c_enum_const() to closely related code

[Eric Blake]

From: Markus Armbruster <armbru@redhat.com>

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi.py | 50 +++++++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3757a91..cc33355 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -742,6 +742,31 @@ def camel_case(name):
             new_name += ch.lower()
     return new_name

+# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
+# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
+# ENUM24_Name -> ENUM24_NAME
+def camel_to_upper(value):
+    c_fun_str = c_name(value, False)
+    if value.isupper():
+        return c_fun_str
+
+    new_name = ''
+    l = len(c_fun_str)
+    for i in range(l):
+        c = c_fun_str[i]
+        # When c is upper and no "_" appears before, do more checks
+        if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
+            # Case 1: next string is lower
+            # Case 2: previous string is digit
+            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
+            c_fun_str[i - 1].isdigit():
+                new_name += '_'
+        new_name += c
+    return new_name.lstrip('_').upper()
+
+def c_enum_const(type_name, const_name):
+    return camel_to_upper(type_name + '_' + const_name)
+
 c_name_trans = string.maketrans('.-', '__')

 def c_name(name, protect=True):
@@ -926,28 +951,3 @@ def guardend(name):

 ''',
                  name=guardname(name))
-
-# ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
-# ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
-# ENUM24_Name -> ENUM24_NAME
-def camel_to_upper(value):
-    c_fun_str = c_name(value, False)
-    if value.isupper():
-        return c_fun_str
-
-    new_name = ''
-    l = len(c_fun_str)
-    for i in range(l):
-        c = c_fun_str[i]
-        # When c is upper and no "_" appears before, do more checks
-        if c.isupper() and (i > 0) and c_fun_str[i - 1] != "_":
-            # Case 1: next string is lower
-            # Case 2: previous string is digit
-            if (i < (l - 1) and c_fun_str[i + 1].islower()) or \
-            c_fun_str[i - 1].isdigit():
-                new_name += '_'
-        new_name += c
-    return new_name.lstrip('_').upper()
-
-def c_enum_const(type_name, const_name):
-    return camel_to_upper(type_name + '_' + const_name)
-- 
2.1.0

[Qemu-devel] [PATCH v4 08/16] qapi: Tidy c_type logic

[Eric Blake]

c_type() is designed to be called on both string names and on
array designations, so 'name' is a bit misleading because it
operates on more than strings.  Also, no caller ever passes
an empty string.  Finally, + notation is a bit nicer to read
than '%s' % value for string concatenation.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v4: new patch, suggested by Markus
---
 scripts/qapi.py | 58 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 28 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index cc33355..85e5d00 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -801,12 +801,12 @@ def c_name(name, protect=True):
     return name.translate(c_name_trans)

 def c_list_type(name):
-    return '%sList' % name
+    return name + 'List'

-def type_name(name):
-    if type(name) == list:
-        return c_list_type(name[0])
-    return name
+def type_name(value):
+    if type(value) == list:
+        return c_list_type(value[0])
+    return value

 def add_name(name, info, meta, implicit = False):
     global all_names
@@ -863,42 +863,44 @@ def is_enum(name):
     return find_enum(name) != None

 eatspace = '\033EATSPACE.'
+pointer_suffix = ' *' + eatspace

 # A special suffix is added in c_type() for pointer types, and it's
 # stripped in mcgen(). So please notice this when you check the return
 # value of c_type() outside mcgen().
-def c_type(name, is_param=False):
-    if name == 'str':
+def c_type(value, is_param=False):
+    if value == 'str':
         if is_param:
-            return 'const char *' + eatspace
-        return 'char *' + eatspace
+            return 'const char' + pointer_suffix
+        return 'char' + pointer_suffix

-    elif name == 'int':
+    elif value == 'int':
         return 'int64_t'
-    elif (name == 'int8' or name == 'int16' or name == 'int32' or
-          name == 'int64' or name == 'uint8' or name == 'uint16' or
-          name == 'uint32' or name == 'uint64'):
-        return name + '_t'
-    elif name == 'size':
+    elif (value == 'int8' or value == 'int16' or value == 'int32' or
+          value == 'int64' or value == 'uint8' or value == 'uint16' or
+          value == 'uint32' or value == 'uint64'):
+        return value + '_t'
+    elif value == 'size':
         return 'uint64_t'
-    elif name == 'bool':
+    elif value == 'bool':
         return 'bool'
-    elif name == 'number':
+    elif value == 'number':
         return 'double'
-    elif type(name) == list:
-        return '%s *%s' % (c_list_type(name[0]), eatspace)
-    elif is_enum(name):
-        return name
-    elif name == None or len(name) == 0:
+    elif type(value) == list:
+        return c_list_type(value[0]) + pointer_suffix
+    elif is_enum(value):
+        return value
+    elif value == None:
         return 'void'
-    elif name in events:
-        return '%sEvent *%s' % (camel_case(name), eatspace)
+    elif value in events:
+        return camel_case(value) + 'Event' + pointer_suffix
     else:
-        return '%s *%s' % (name, eatspace)
+        # complex type name
+        assert isinstance(value, str) and str != ""
+        return value + pointer_suffix

-def is_c_ptr(name):
-    suffix = "*" + eatspace
-    return c_type(name).endswith(suffix)
+def is_c_ptr(value):
+    return c_type(value).endswith(pointer_suffix)

 def genindent(count):
     ret = ""
-- 
2.1.0

[Qemu-devel] [PATCH v4 09/16] qapi: Make c_type() consistently convert qapi names

[Eric Blake]

Continuing the string of cleanups for supporting downstream names
containing '.', this patch focuses on ensuring c_type() can
handle a downstream name.  This patch alone does not fix the
places where generator output should be calling this function
but was open-coding things instead, but it gets us a step closer.

In particular, the changes to c_list_type() and type_name() mean
that type_name(FOO) now handles the case when FOO contains '.',
'-', or is a ticklish identifier other than a builtin (builtins
are exempted because ['int'] must remain mapped to 'intList' and
not 'q_intList').  Meanwhile, ['unix'] now maps to 'q_unixList'
rather than 'unixList', to match the fact that 'unix' is ticklish;
however, our naming conventions state that complex types should
start with a capital, so no type name following conventions will
ever have the 'q_' prepended.

Likewise, changes to c_type() mean that c_type(FOO) properly
handles an enum or complex type FOO with '.' or '-' in the
name, or is a ticklish identifier (again, a ticklish identifier
as a type name violates conventions).

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v4: more comments based on feedback from Markus; rebase atop
inserted patch with parameter rename
---
 scripts/qapi.py | 33 +++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 85e5d00..309dfec 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -769,6 +769,15 @@ def c_enum_const(type_name, const_name):

 c_name_trans = string.maketrans('.-', '__')

+# Map @name to a valid C identifier.
+# If @protect, avoid returning certain ticklish identifiers (like
+# C keywords) by prepending "q_".
+#
+# Used for converting 'name' from a 'name':'type' qapi definition
+# into a generated struct member, as well as converting type names
+# into substrings of a generated C function name.
+# '__a.b_c' -> '__a_b_c', 'x-foo' -> 'x_foo'
+# protect=True: 'int' -> 'q_int'; protect=False: 'int' -> 'int'
 def c_name(name, protect=True):
     # ANSI X3J11/88-090, 3.1.1
     c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
@@ -800,13 +809,25 @@ def c_name(name, protect=True):
         return "q_" + name
     return name.translate(c_name_trans)

+# Map type @name to the C typedef name for the list form.
+#
+# ['Name'] -> 'NameList', ['x-Foo'] -> 'x_FooList', ['int'] -> 'intList'
 def c_list_type(name):
-    return name + 'List'
+    return type_name(name) + 'List'

+# Map type @value to the C typedef form.
+#
+# Used for converting 'type' from a 'member':'type' qapi definition
+# into the alphanumeric portion of the type for a generated C parameter,
+# as well as generated C function names.  See c_type() for the rest of
+# the conversion such as adding '*' on pointer types.
+# 'int' -> 'int', '[x-Foo]' -> 'x_FooList', '__a.b_c' -> '__a_b_c'
 def type_name(value):
     if type(value) == list:
         return c_list_type(value[0])
-    return value
+    if value in builtin_types.keys():
+        return value
+    return c_name(value)

 def add_name(name, info, meta, implicit = False):
     global all_names
@@ -865,6 +886,10 @@ def is_enum(name):
 eatspace = '\033EATSPACE.'
 pointer_suffix = ' *' + eatspace

+# Map type @name to its C type expression.
+# If @is_param, const-qualify the string type.
+#
+# This function is used for computing the full C type of 'member':'name'.
 # A special suffix is added in c_type() for pointer types, and it's
 # stripped in mcgen(). So please notice this when you check the return
 # value of c_type() outside mcgen().
@@ -889,7 +914,7 @@ def c_type(value, is_param=False):
     elif type(value) == list:
         return c_list_type(value[0]) + pointer_suffix
     elif is_enum(value):
-        return value
+        return c_name(value)
     elif value == None:
         return 'void'
     elif value in events:
@@ -897,7 +922,7 @@ def c_type(value, is_param=False):
     else:
         # complex type name
         assert isinstance(value, str) and str != ""
-        return value + pointer_suffix
+        return c_name(value) + pointer_suffix

 def is_c_ptr(value):
     return c_type(value).endswith(pointer_suffix)
-- 
2.1.0

[Qemu-devel] [PATCH v4 10/16] qapi: Support downstream enums

[Eric Blake]

Enhance the testsuite to cover a downstream enum type and enum
string.  Update the generator to mangle the enum name in the
appropriate places.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v4: use type_name() instead of special-casing builtins in
generate_visit_list()
---
 scripts/qapi-types.py                   | 15 ++++++++-------
 scripts/qapi-visit.py                   |  8 ++++----
 tests/qapi-schema/qapi-schema-test.json |  3 +++
 tests/qapi-schema/qapi-schema-test.out  |  4 +++-
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 9eb08a6..1593fc6 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -58,7 +58,7 @@ typedef struct %(name)sList
     struct %(name)sList *next;
 } %(name)sList;
 ''',
-                 name=name)
+                 name=c_name(name))

 def generate_struct_fields(members):
     ret = ''
@@ -115,7 +115,7 @@ def generate_enum_lookup(name, values):
     ret = mcgen('''
 const char *%(name)s_lookup[] = {
 ''',
-                         name=name)
+                name=c_name(name))
     i = 0
     for value in values:
         index = c_enum_const(name, value)
@@ -134,6 +134,7 @@ const char *%(name)s_lookup[] = {
     return ret

 def generate_enum(name, values):
+    name = c_name(name)
     lookup_decl = mcgen('''
 extern const char *%(name)s_lookup[];
 ''',
@@ -247,15 +248,15 @@ extern const int %(name)s_qtypes[];

 def generate_type_cleanup_decl(name):
     ret = mcgen('''
-void qapi_free_%(type)s(%(c_type)s obj);
+void qapi_free_%(name)s(%(c_type)s obj);
 ''',
-                c_type=c_type(name),type=name)
+                c_type=c_type(name), name=c_name(name))
     return ret

 def generate_type_cleanup(name):
     ret = mcgen('''

-void qapi_free_%(type)s(%(c_type)s obj)
+void qapi_free_%(name)s(%(c_type)s obj)
 {
     QapiDeallocVisitor *md;
     Visitor *v;
@@ -266,11 +267,11 @@ void qapi_free_%(type)s(%(c_type)s obj)

     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
-    visit_type_%(type)s(v, &obj, NULL, NULL);
+    visit_type_%(name)s(v, &obj, NULL, NULL);
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
-                c_type=c_type(name),type=name)
+                c_type=c_type(name), name=c_name(name))
     return ret


diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0368e62..7697ec6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -173,7 +173,7 @@ out:
     error_propagate(errp, err);
 }
 ''',
-                name=name)
+                name=type_name(name))

 def generate_visit_enum(name, members):
     return mcgen('''
@@ -183,7 +183,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **er
     visit_type_enum(m, (int *)obj, %(name)s_lookup, "%(name)s", name, errp);
 }
 ''',
-                 name=name)
+                 name=c_name(name))

 def generate_visit_alternate(name, members):
     ret = mcgen('''
@@ -364,7 +364,7 @@ def generate_enum_declaration(name, members):
     ret = mcgen('''
 void visit_type_%(name)sList(Visitor *m, %(name)sList **obj, const char *name, Error **errp);
 ''',
-                name=name)
+                name=c_name(name))

     return ret

@@ -373,7 +373,7 @@ def generate_decl_enum(name, members):

 void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **errp);
 ''',
-                name=name)
+                 name=c_name(name))

 try:
     opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:i:o:",
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 8193dc1..5f9af66 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -107,3 +107,6 @@
   'data': { '*a': 'int', '*b': 'UserDefOne', 'c': 'str' } }
 { 'event': 'EVENT_D',
   'data': { 'a' : 'EventStructOne', 'b' : 'str', '*c': 'str', '*enum3': 'EnumOne' } }
+
+# test that we correctly compile downstream extensions
+{ 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 93c4963..40f0f20 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -22,8 +22,10 @@
  OrderedDict([('event', 'EVENT_A')]),
  OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
  OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]),
- OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])]
+ OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
+ OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
  {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
 [OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
-- 
2.1.0

[Qemu-devel] [PATCH v4 11/16] qapi: Support downstream structs

[Eric Blake]

Enhance the testsuite to cover downstream structs, including struct
members and base structs.  Update the generator to mangle the
struct names in the appropriate places.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py                   |  4 ++--
 scripts/qapi-visit.py                   | 11 ++++++-----
 tests/qapi-schema/qapi-schema-test.json |  4 ++++
 tests/qapi-schema/qapi-schema-test.out  |  8 ++++++--
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 1593fc6..5118151 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -45,7 +45,7 @@ typedef struct %(name)sList
     struct %(name)sList *next;
 } %(name)sList;
 ''',
-                 name=name)
+                 name=c_name(name))

 def generate_fwd_enum_struct(name, members):
     return mcgen('''
@@ -87,7 +87,7 @@ def generate_struct(expr):
 struct %(name)s
 {
 ''',
-          name=structname)
+          name=c_name(structname))

     if base:
         ret += generate_struct_fields({'base': base})
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 7697ec6..b500724 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -56,7 +56,7 @@ static void visit_type_%(name)s_fields(Visitor *m, %(name)s **obj, Error **errp)
 {
     Error *err = NULL;
 ''',
-                 name=name)
+                 name=c_name(name))
     push_indent()

     if base:
@@ -111,16 +111,16 @@ def generate_visit_struct_body(name, members):
     ret = mcgen('''
     Error *err = NULL;

-    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(name)s), &err);
+    visit_start_struct(m, (void **)obj, "%(name)s", name, sizeof(%(c_name)s), &err);
     if (!err) {
         if (*obj) {
-            visit_type_%(name)s_fields(m, obj, errp);
+            visit_type_%(c_name)s_fields(m, obj, errp);
         }
         visit_end_struct(m, &err);
     }
     error_propagate(errp, err);
 ''',
-                name=name)
+                name=name, c_name=c_name(name))

     return ret

@@ -137,7 +137,7 @@ def generate_visit_struct(expr):
 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp)
 {
 ''',
-                name=name)
+                 name=c_name(name))

     ret += generate_visit_struct_body(name, members)

@@ -347,6 +347,7 @@ out:
 def generate_declaration(name, members, builtin_type=False):
     ret = ""
     if not builtin_type:
+        name = c_name(name)
         ret += mcgen('''

 void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp);
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 5f9af66..ca9b34c 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -110,3 +110,7 @@

 # test that we correctly compile downstream extensions
 { 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }
+{ 'struct': '__org.qemu_x-Base',
+  'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
+{ 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
+  'data': { '__org.qemu_x-member2': 'str' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 40f0f20..6ab4f00 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -23,7 +23,9 @@
  OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]),
  OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'), ('*b', 'UserDefOne'), ('c', 'str')]))]),
  OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
- OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])])]
+ OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])]),
+ OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
@@ -39,4 +41,6 @@
  OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
- OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))])]
+ OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))])]
-- 
2.1.0

[Qemu-devel] [PATCH v4 12/16] qapi: Support downstream simple unions

[Eric Blake]

Enhance the testsuite to cover downstream simple unions, including
when a union branch is a downstream name.  Update the generator to
mangle the union names in the appropriate places.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py                   | 2 +-
 scripts/qapi-visit.py                   | 8 ++++----
 tests/qapi-schema/qapi-schema-test.json | 1 +
 tests/qapi-schema/qapi-schema-test.out  | 6 ++++--
 4 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5118151..5b0bc5d 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -193,7 +193,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = {

 def generate_union(expr, meta):

-    name = expr[meta]
+    name = c_name(expr[meta])
     typeinfo = expr['data']

     base = expr.get('base')
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index b500724..d1ec70b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -255,9 +255,9 @@ def generate_visit_union(expr):
         disc_type = enum_define['enum_name']
     else:
         # There will always be a discriminator in the C switch code, by default
-        # it is an enum type generated silently as "'%sKind' % (name)"
-        ret = generate_visit_enum('%sKind' % name, members.keys())
-        disc_type = '%sKind' % (name)
+        # it is an enum type generated silently
+        ret = generate_visit_enum(name + 'Kind', members.keys())
+        disc_type = c_name(name) + 'Kind'

     if base:
         assert discriminator
@@ -281,7 +281,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
     }
     if (*obj) {
 ''',
-                 name=name)
+                 name=c_name(name))

     if base:
         ret += mcgen('''
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ca9b34c..6416d85 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -114,3 +114,4 @@
   'data': { '__org.qemu_x-member1': '__org.qemu_x-Enum' } }
 { 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
   'data': { '__org.qemu_x-member2': 'str' } }
+{ 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 6ab4f00..f9ebe08 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -25,11 +25,13 @@
  OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a', 'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))]),
  OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])]),
  OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))])]
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
+ OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
- {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None},
+ {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None}]
 [OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
  OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
-- 
2.1.0

[Qemu-devel] [PATCH v4 13/16] qapi: Support downstream flat unions

[Eric Blake]

Enhance the testsuite to cover downstream flat unions, including
the base type, discriminator name and type, and branch name and
type.  Update the generator to mangle the union names in the
appropriate places.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py                   | 2 +-
 scripts/qapi-visit.py                   | 4 ++--
 tests/qapi-schema/qapi-schema-test.json | 5 +++++
 tests/qapi-schema/qapi-schema-test.out  | 7 +++++--
 4 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5b0bc5d..13e4b53 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -213,7 +213,7 @@ struct %(name)s
         void *data;
 ''',
                 name=name,
-                discriminator_type_name=discriminator_type_name)
+                discriminator_type_name=c_name(discriminator_type_name))

     for key in typeinfo:
         ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d1ec70b..c15305f 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -252,7 +252,7 @@ def generate_visit_union(expr):
     if enum_define:
         # Use the enum type as discriminator
         ret = ""
-        disc_type = enum_define['enum_name']
+        disc_type = c_name(enum_define['enum_name'])
     else:
         # There will always be a discriminator in the C switch code, by default
         # it is an enum type generated silently
@@ -290,7 +290,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
             goto out_obj;
         }
 ''',
-            name=name)
+                     name=c_name(name))

     if not discriminator:
         disc_key = "type"
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 6416d85..ac236e3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -115,3 +115,8 @@
 { 'struct': '__org.qemu_x-Struct', 'base': '__org.qemu_x-Base',
   'data': { '__org.qemu_x-member2': 'str' } }
 { 'union': '__org.qemu_x-Union1', 'data': { '__org.qemu_x-branch': 'str' } }
+{ 'struct': '__org.qemu_x-Struct2',
+  'data': { 'array': ['__org.qemu_x-Union1'] } }
+{ 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
+  'discriminator': '__org.qemu_x-member1',
+  'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index f9ebe08..3fc24e8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -26,7 +26,9 @@
  OrderedDict([('enum', '__org.qemu_x-Enum'), ('data', ['__org.qemu_x-value'])]),
  OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
  OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
- OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))])]
+ OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
+ OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'), ('discriminator', '__org.qemu_x-member1'), ('data', OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
@@ -45,4 +47,5 @@
  OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
  OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]),
  OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]),
- OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))])]
+ OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
+ OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))])]
-- 
2.1.0

[Qemu-devel] [PATCH v4 14/16] qapi: Support downstream alternates

[Eric Blake]

Enhance the testsuite to cover downstream alternates, including
whether the branch name or type is downstream.  Update the
generator to mangle alternate names in the appropriate places.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py                   | 7 ++++---
 scripts/qapi-visit.py                   | 6 +++---
 tests/qapi-schema/qapi-schema-test.json | 2 ++
 tests/qapi-schema/qapi-schema-test.out  | 6 ++++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 13e4b53..5665145 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -174,16 +174,17 @@ def generate_alternate_qtypes(expr):
     ret = mcgen('''
 const int %(name)s_qtypes[QTYPE_MAX] = {
 ''',
-    name=name)
+                name=c_name(name))

     for key in members:
         qtype = find_alternate_member_qtype(members[key])
         assert qtype, "Invalid alternate member"

         ret += mcgen('''
-    [ %(qtype)s ] = %(enum_const)s,
+    [%(qtype)s] = %(enum_const)s,
 ''',
-        qtype = qtype, enum_const = c_enum_const(name + 'Kind', key))
+                     qtype = qtype,
+                     enum_const = c_enum_const(name + 'Kind', key))

     ret += mcgen('''
 };
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c15305f..e511be3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -202,11 +202,11 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
     }
     switch ((*obj)->kind) {
 ''',
-    name=name)
+                name=c_name(name))

     # For alternate, always use the default enum type automatically generated
-    # as "'%sKind' % (name)"
-    disc_type = '%sKind' % (name)
+    # as name + 'Kind'
+    disc_type = c_name(name) + 'Kind'

     for key in members:
         assert (members[key] in builtin_types.keys()
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ac236e3..d586b56 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -120,3 +120,5 @@
 { 'union': '__org.qemu_x-Union2', 'base': '__org.qemu_x-Base',
   'discriminator': '__org.qemu_x-member1',
   'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
+{ 'alternate': '__org.qemu_x-Alt',
+  'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 3fc24e8..2161a90 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -28,12 +28,14 @@
  OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]),
  OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))]),
  OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
- OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'), ('discriminator', '__org.qemu_x-member1'), ('data', OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))])]
+ OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'), ('discriminator', '__org.qemu_x-member1'), ('data', OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))]),
+ OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data', OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
  {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None},
- {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None}]
+ {'enum_name': '__org.qemu_x-Union1Kind', 'enum_values': None},
+ {'enum_name': '__org.qemu_x-AltKind', 'enum_values': None}]
 [OrderedDict([('struct', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
  OrderedDict([('struct', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
  OrderedDict([('struct', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
-- 
2.1.0

[Qemu-devel] [PATCH v4 15/16] qapi: Support downstream events and commands

[Eric Blake]

Enhance the testsuite to cover downstream events and commands.
Events worked without more tweaks, but commands needed a few final
updates in the generator to mangle names in the appropriate places.
In making those tweaks, it was easier to drop type_visitor() and
inline its actions instead.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-commands.py                | 16 +++++-----------
 tests/qapi-schema/qapi-schema-test.json |  5 +++++
 tests/qapi-schema/qapi-schema-test.out  |  4 +++-
 tests/test-qmp-commands.c               | 15 +++++++++++++++
 4 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 8c125ca..0a1d636 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -20,12 +20,6 @@ import os
 import getopt
 import errno

-def type_visitor(name):
-    if type(name) == list:
-        return 'visit_type_%sList' % name[0]
-    else:
-        return 'visit_type_%s' % name
-
 def generate_command_decl(name, args, ret_type):
     arglist=""
     for argname, argtype, optional in parse_args(args):
@@ -153,10 +147,10 @@ if (has_%(c_name)s) {
                          c_name=c_name(argname))
             push_indent()
         ret += mcgen('''
-%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
+visit_type_%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_name=c_name(argname), name=argname, argtype=argtype,
-                     visitor=type_visitor(argtype), errp=errparg)
+                     visitor=type_name(argtype), errp=errparg)
         ret += gen_err_check(errarg)
         if optional:
             pop_indent()
@@ -184,7 +178,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o
     Visitor *v;

     v = qmp_output_get_visitor(mo);
-    %(visitor)s(v, &ret_in, "unused", &local_err);
+    visit_type_%(visitor)s(v, &ret_in, "unused", &local_err);
     if (local_err) {
         goto out;
     }
@@ -195,12 +189,12 @@ out:
     qmp_output_visitor_cleanup(mo);
     md = qapi_dealloc_visitor_new();
     v = qapi_dealloc_get_visitor(md);
-    %(visitor)s(v, &ret_in, "unused", NULL);
+    visit_type_%(visitor)s(v, &ret_in, "unused", NULL);
     qapi_dealloc_visitor_cleanup(md);
 }
 ''',
                 c_ret_type=c_type(ret_type), c_name=c_name(name),
-                visitor=type_visitor(ret_type))
+                visitor=type_name(ret_type))

     return ret

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index d586b56..c7eaa86 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -122,3 +122,8 @@
   'data': { '__org.qemu_x-value': '__org.qemu_x-Struct2' } }
 { 'alternate': '__org.qemu_x-Alt',
   'data': { '__org.qemu_x-branch': 'str', 'b': '__org.qemu_x-Base' } }
+{ 'event': '__ORG.QEMU_X-EVENT', 'data': '__org.qemu_x-Struct' }
+{ 'command': '__org.qemu_x-command',
+  'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'],
+            'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' },
+  'returns': '__org.qemu_x-Union1' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 2161a90..cf0ccc4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -29,7 +29,9 @@
  OrderedDict([('union', '__org.qemu_x-Union1'), ('data', OrderedDict([('__org.qemu_x-branch', 'str')]))]),
  OrderedDict([('struct', '__org.qemu_x-Struct2'), ('data', OrderedDict([('array', ['__org.qemu_x-Union1'])]))]),
  OrderedDict([('union', '__org.qemu_x-Union2'), ('base', '__org.qemu_x-Base'), ('discriminator', '__org.qemu_x-member1'), ('data', OrderedDict([('__org.qemu_x-value', '__org.qemu_x-Struct2')]))]),
- OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data', OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))])]
+ OrderedDict([('alternate', '__org.qemu_x-Alt'), ('data', OrderedDict([('__org.qemu_x-branch', 'str'), ('b', '__org.qemu_x-Base')]))]),
+ OrderedDict([('event', '__ORG.QEMU_X-EVENT'), ('data', '__org.qemu_x-Struct')]),
+ OrderedDict([('command', '__org.qemu_x-command'), ('data', OrderedDict([('a', ['__org.qemu_x-Enum']), ('b', ['__org.qemu_x-Struct']), ('c', '__org.qemu_x-Union2'), ('d', '__org.qemu_x-Alt')])), ('returns', '__org.qemu_x-Union1')])]
 [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
  {'enum_name': '__org.qemu_x-Enum', 'enum_values': ['__org.qemu_x-value']},
  {'enum_name': 'UserDefAlternateKind', 'enum_values': None},
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index ad2e403..9918f23 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -51,6 +51,21 @@ int64_t qmp_user_def_cmd3(int64_t a, bool has_b, int64_t b, Error **errp)
     return a + (has_b ? b : 0);
 }

+__org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
+                                              __org_qemu_x_StructList *b,
+                                              __org_qemu_x_Union2 *c,
+                                              __org_qemu_x_Alt *d,
+                                              Error **errp)
+{
+    __org_qemu_x_Union1 *ret = g_new0(__org_qemu_x_Union1, 1);
+
+    ret->kind = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
+    ret->__org_qemu_x_branch = strdup("blah1");
+
+    return ret;
+}
+
+
 /* test commands with no input and no return value */
 static void test_dispatch_cmd(void)
 {
-- 
2.1.0

[Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var'

[Eric Blake]

Use of python's % operator to format strings is fine if there are
multiple strings or if there is integer formatting going on, but
when it is just abused for string concatenation, it looks nicer
to just use the + operator.  This is particularly true when the
value being substituted is at the front of the format string,
rather than the tail.

Update an error message (and testsuite coverage) while at it, since
concatenating a non-string object does not always produce legible
results.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

v4: new patch
---
 scripts/qapi-commands.py               | 17 +++++----
 scripts/qapi-event.py                  |  2 +-
 scripts/qapi-types.py                  | 10 +++---
 scripts/qapi-visit.py                  |  4 +--
 scripts/qapi.py                        | 64 +++++++++++++++++-----------------
 tests/qapi-schema/include-non-file.err |  2 +-
 6 files changed, 51 insertions(+), 48 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0a1d636..1464a3d 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -25,7 +25,7 @@ def generate_command_decl(name, args, ret_type):
     for argname, argtype, optional in parse_args(args):
         argtype = c_type(argtype, is_param=True)
         if optional:
-            arglist += "bool has_%s, " % c_name(argname)
+            arglist += "bool has_" + c_name(argname) + ", "
         arglist += "%s %s, " % (argtype, c_name(argname))
     return mcgen('''
 %(ret_type)s qmp_%(name)s(%(args)sError **errp);
@@ -50,8 +50,8 @@ def gen_sync_call(name, args, ret_type, indent=0):
         retval = "retval = "
     for argname, argtype, optional in parse_args(args):
         if optional:
-            arglist += "has_%s, " % c_name(argname)
-        arglist += "%s, " % (c_name(argname))
+            arglist += "has_" + c_name(argname) + ", "
+        arglist += c_name(argname) + ", "
     push_indent(indent)
     ret = mcgen('''
 %(retval)sqmp_%(name)s(%(args)s&local_err);
@@ -71,7 +71,7 @@ def gen_sync_call(name, args, ret_type, indent=0):
 def gen_marshal_output_call(name, ret_type):
     if not ret_type:
         return ""
-    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
+    return "qmp_marshal_output_" + c_name(name) + "(retval, ret, &local_err);"

 def gen_visitor_input_containers_decl(args, obj):
     ret = ""
@@ -200,9 +200,11 @@ out:

 def gen_marshal_input_decl(name, args, ret_type, middle_mode):
     if middle_mode:
-        return 'int qmp_marshal_input_%s(Monitor *mon, const QDict *qdict, QObject **ret)' % c_name(name)
+        return ('int qmp_marshal_input_' + c_name(name)
+                + '(Monitor *mon, const QDict *qdict, QObject **ret)')
     else:
-        return 'static void qmp_marshal_input_%s(QDict *args, QObject **ret, Error **errp)' % c_name(name)
+        return ('static void qmp_marshal_input_' + c_name(name)
+                + '(QDict *args, QObject **ret, Error **errp)')



@@ -458,7 +460,8 @@ if dispatch_type == "sync":
             fdef.write(ret)

         if middle_mode:
-            fdecl.write('%s;\n' % gen_marshal_input_decl(cmd['command'], arglist, ret_type, middle_mode))
+            fdecl.write(gen_marshal_input_decl(cmd['command'], arglist,
+                                               ret_type, middle_mode) + ';\n')

         ret = gen_marshal_input(cmd['command'], arglist, ret_type, middle_mode) + "\n"
         fdef.write(ret)
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index a7e0033..957dba5 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -17,7 +17,7 @@ import getopt
 import errno

 def _generate_event_api_name(event_name, params):
-    api_name = "void qapi_event_send_%s(" % c_name(event_name).lower();
+    api_name = "void qapi_event_send_" + c_name(event_name).lower() + "(";
     l = len(api_name)

     if params:
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5665145..1c41743 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -204,7 +204,7 @@ def generate_union(expr, meta):
     if enum_define:
         discriminator_type_name = enum_define['enum_name']
     else:
-        discriminator_type_name = '%sKind' % (name)
+        discriminator_type_name = name + 'Kind'

     ret = mcgen('''
 struct %(name)s
@@ -399,13 +399,13 @@ for expr in exprs:
         ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
         enum_define = discriminator_find_enum_define(expr)
         if not enum_define:
-            ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
-            fdef.write(generate_enum_lookup('%sKind' % expr['union'],
+            ret += generate_enum(expr['union'] + 'Kind', expr['data'].keys())
+            fdef.write(generate_enum_lookup(expr['union'] + 'Kind',
                                             expr['data'].keys()))
     elif expr.has_key('alternate'):
         ret += generate_fwd_struct(expr['alternate'], expr['data']) + "\n"
-        ret += generate_enum('%sKind' % expr['alternate'], expr['data'].keys())
-        fdef.write(generate_enum_lookup('%sKind' % expr['alternate'],
+        ret += generate_enum(expr['alternate'] + 'Kind', expr['data'].keys())
+        fdef.write(generate_enum_lookup(expr['alternate'] + 'Kind',
                                         expr['data'].keys()))
         fdef.write(generate_alternate_qtypes(expr))
     else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e511be3..4287251 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -509,7 +509,7 @@ for expr in exprs:
         enum_define = discriminator_find_enum_define(expr)
         ret = ""
         if not enum_define:
-            ret = generate_decl_enum('%sKind' % expr['union'],
+            ret = generate_decl_enum(expr['union'] + 'Kind',
                                      expr['data'].keys())
         ret += generate_declaration(expr['union'], expr['data'])
         fdecl.write(ret)
@@ -518,7 +518,7 @@ for expr in exprs:
         ret += generate_visit_list(expr['alternate'], expr['data'])
         fdef.write(ret)

-        ret = generate_decl_enum('%sKind' % expr['alternate'],
+        ret = generate_decl_enum(expr['alternate'] + 'Kind',
                                  expr['data'].keys())
         ret += generate_declaration(expr['alternate'], expr['data'])
         fdecl.write(ret)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 309dfec..1a11f61 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -122,21 +122,22 @@ class QAPISchema:
         self.accept()

         while self.tok != None:
-            expr_info = {'file': input_relname, 'line': self.line, 'parent': self.parent_info}
+            expr_info = {'file': input_relname, 'line': self.line,
+                         'parent': self.parent_info}
             expr = self.get_expr(False)
             if isinstance(expr, dict) and "include" in expr:
                 if len(expr) != 1:
-                    raise QAPIExprError(expr_info, "Invalid 'include' directive")
+                    raise QAPIExprError(expr_info,
+                                        "Invalid 'include' directive")
                 include = expr["include"]
                 if not isinstance(include, str):
                     raise QAPIExprError(expr_info,
-                                        'Expected a file name (string), got: %s'
-                                        % include)
+                                        "Expected a string for 'include' value")
                 include_path = os.path.join(self.input_dir, include)
                 for elem in self.include_hist:
                     if include_path == elem[1]:
-                        raise QAPIExprError(expr_info, "Inclusion loop for %s"
-                                            % include)
+                        raise QAPIExprError(expr_info,
+                                            "Inclusion loop for " + include)
                 # skip multiple include of the same file
                 if include_path in previously_included:
                     continue
@@ -208,7 +209,7 @@ class QAPISchema:
                             string += ch
                         else:
                             raise QAPISchemaError(self,
-                                                  "Unknown escape \\%s" %ch)
+                                                  "Unknown escape \\" + ch)
                         esc = False
                     elif ch == "\\":
                         esc = True
@@ -254,7 +255,7 @@ class QAPISchema:
                 raise QAPISchemaError(self, 'Expected ":"')
             self.accept()
             if key in expr:
-                raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
+                raise QAPISchemaError(self, 'Duplicate key "' + key + '"')
             expr[key] = self.get_expr(True)
             if self.tok == '}':
                 self.accept()
@@ -343,7 +344,7 @@ def check_name(expr_info, source, name, allow_optional = False,

     if not isinstance(name, str):
         raise QAPIExprError(expr_info,
-                            "%s requires a string name" % source)
+                            source + " requires a string name")
     if name.startswith('*'):
         membername = name[1:]
         if not allow_optional:
@@ -374,13 +375,12 @@ def check_type(expr_info, source, value, allow_array = False,
     if isinstance(value, list):
         if not allow_array:
             raise QAPIExprError(expr_info,
-                                "%s cannot be an array" % source)
+                                source + " cannot be an array")
         if len(value) != 1 or not isinstance(value[0], str):
-            raise QAPIExprError(expr_info,
-                                "%s: array type must contain single type name"
-                                % source)
+            raise QAPIExprError(expr_info, source +
+                                ": array type must contain single type name")
         value = value[0]
-        orig_value = "array of %s" %value
+        orig_value = "array of " + value

     # Check if type name for value is okay
     if isinstance(value, str):
@@ -401,12 +401,12 @@ def check_type(expr_info, source, value, allow_array = False,
     # value is a dictionary, check that each member is okay
     if not isinstance(value, OrderedDict):
         raise QAPIExprError(expr_info,
-                            "%s should be a dictionary" % source)
+                            source + " should be a dictionary")
     if not allow_dict:
         raise QAPIExprError(expr_info,
-                            "%s should be a type name" % source)
+                            source + " should be a type name")
     for (key, arg) in value.items():
-        check_name(expr_info, "Member of %s" % source, key,
+        check_name(expr_info, "Member of " + source, key,
                    allow_optional=allow_optional)
         # Todo: allow dictionaries to represent default values of
         # an optional argument.
@@ -433,13 +433,13 @@ def check_command(expr, expr_info):
     name = expr['command']
     allow_star = expr.has_key('gen')

-    check_type(expr_info, "'data' for command '%s'" % name,
+    check_type(expr_info, "'data' for command '" + name + "'",
                expr.get('data'), allow_dict=True, allow_optional=True,
                allow_metas=['union', 'struct'], allow_star=allow_star)
     returns_meta = ['union', 'struct']
     if name in returns_whitelist:
         returns_meta += ['built-in', 'alternate', 'enum']
-    check_type(expr_info, "'returns' for command '%s'" % name,
+    check_type(expr_info, "'returns' for command '" + name + "'",
                expr.get('returns'), allow_array=True, allow_dict=True,
                allow_optional=True, allow_metas=returns_meta,
                allow_star=allow_star)
@@ -452,7 +452,7 @@ def check_event(expr, expr_info):
     if name.upper() == 'MAX':
         raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
     events.append(name)
-    check_type(expr_info, "'data' for event '%s'" % name,
+    check_type(expr_info, "'data' for event '" + name + "'",
                expr.get('data'), allow_dict=True, allow_optional=True,
                allow_metas=['union', 'struct'])

@@ -469,7 +469,7 @@ def check_union(expr, expr_info):
         if discriminator is None:
             raise QAPIExprError(expr_info,
                                 "Union '%s' requires a discriminator to go "
-                                "along with base" %name)
+                                "along with base" % name)

     # Two types of unions, determined by discriminator.

@@ -497,7 +497,7 @@ def check_union(expr, expr_info):

         # The value of member 'discriminator' must name a non-optional
         # member of the base struct.
-        check_name(expr_info, "Discriminator of flat union '%s'" % name,
+        check_name(expr_info, "Discriminator of flat union '" + name + "'",
                    discriminator)
         discriminator_type = base_fields.get(discriminator)
         if not discriminator_type:
@@ -515,7 +515,7 @@ def check_union(expr, expr_info):

     # Check every branch
     for (key, value) in members.items():
-        check_name(expr_info, "Member of union '%s'" % name, key)
+        check_name(expr_info, "Member of union '" + name + "'", key)

         # Each value must name a known type; furthermore, in flat unions,
         # branches must be a struct with no overlapping member names
@@ -525,7 +525,7 @@ def check_union(expr, expr_info):
             branch_struct = find_struct(value)
             assert branch_struct
             check_member_clash(expr_info, base, branch_struct['data'],
-                               " of branch '%s'" % key)
+                               " of branch '" + key + "'")

         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
@@ -533,8 +533,8 @@ def check_union(expr, expr_info):
             if not key in enum_define['enum_values']:
                 raise QAPIExprError(expr_info,
                                     "Discriminator value '%s' is not found in "
-                                    "enum '%s'" %
-                                    (key, enum_define["enum_name"]))
+                                    "enum '%s'"
+                                    % (key, enum_define["enum_name"]))

         # Otherwise, check for conflicts in the generated enum
         else:
@@ -585,7 +585,7 @@ def check_enum(expr, expr_info):
         raise QAPIExprError(expr_info,
                             "Enum '%s' requires an array for 'data'" % name)
     for member in members:
-        check_name(expr_info, "Member of enum '%s'" %name, member,
+        check_name(expr_info, "Member of enum '" + name + "'", member,
                    enum_member=True)
         key = camel_to_upper(member)
         if key in values:
@@ -598,9 +598,9 @@ def check_struct(expr, expr_info):
     name = expr['struct']
     members = expr['data']

-    check_type(expr_info, "'data' for struct '%s'" % name, members,
+    check_type(expr_info, "'data' for struct '" + name + "'", members,
                allow_dict=True, allow_optional=True)
-    check_type(expr_info, "'base' for struct '%s'" % name, expr.get('base'),
+    check_type(expr_info, "'base' for struct '" + name + "'", expr.get('base'),
                allow_metas=['struct'])
     if expr.get('base'):
         check_member_clash(expr_info, expr['base'], expr['data'])
@@ -698,10 +698,10 @@ def parse_schema(input_file):
             expr = expr_elem['expr']
             if expr.has_key('union'):
                 if not discriminator_find_enum_define(expr):
-                    add_enum('%sKind' % expr['union'], expr_elem['info'],
+                    add_enum(expr['union'] + 'Kind', expr_elem['info'],
                              implicit=True)
             elif expr.has_key('alternate'):
-                add_enum('%sKind' % expr['alternate'], expr_elem['info'],
+                add_enum(expr['alternate'] + 'Kind', expr_elem['info'],
                          implicit=True)

         # Final pass - validate that exprs make sense
@@ -831,7 +831,7 @@ def type_name(value):

 def add_name(name, info, meta, implicit = False):
     global all_names
-    check_name(info, "'%s'" % meta, name)
+    check_name(info, "'" + meta + "'", name)
     if name in all_names:
         raise QAPIExprError(info,
                             "%s '%s' is already defined"
diff --git a/tests/qapi-schema/include-non-file.err b/tests/qapi-schema/include-non-file.err
index 9658c78..da4e257 100644
--- a/tests/qapi-schema/include-non-file.err
+++ b/tests/qapi-schema/include-non-file.err
@@ -1 +1 @@
-tests/qapi-schema/include-non-file.json:1: Expected a file name (string), got: ['foo', 'bar']
+tests/qapi-schema/include-non-file.json:1: Expected a string for 'include' value
-- 
2.1.0

Re: [Qemu-devel] [PATCH v4 00/16] Fix qapi mangling of downstream names

[Eric Blake]

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

On 05/14/2015 06:50 AM, Eric Blake wrote:
> This series makes it possible to use downstream extensions
> (such as __com.redhat_xyz) and temporary names (such as x-foo)
> in every position possible in QAPI schemes, with added tests
> that the generated code still compiles.
> 
> There's still some things we could do to the qapi generator,
> such as normalizing struct member names and C manglings and
> creating named implicit types up front on the initial parse
> rather than multiple times in each backend.  But that should
> wait until existing pending patches have landed, to minimize
> rebase churn.
> 
> v3 was here:
> https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00519.html
> [already queued in Markus tree, but not yet in a pull request, so
> that queue can be ruthlessly rebased]
> 
> v4 includes 2 new patches (although 14/14 is somewhat unrelated,

Make that 16/16 as the new patch that could be deferred to later (I was
looking at the overall size of v3, while writing about v4)

> and could easily be dropped from the series if it needs another
> spin without holding up the rest of the series), incorporates
> Markus' suggestions for more comments, and simplifies special-casing
> of lists of builtin types.
> 


-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 08/16] qapi: Tidy c_type logic

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> c_type() is designed to be called on both string names and on
> array designations, so 'name' is a bit misleading because it
> operates on more than strings.  Also, no caller ever passes
> an empty string.  Finally, + notation is a bit nicer to read
> than '%s' % value for string concatenation.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v4 08/16] qapi: Tidy c_type logic

[Eric Blake]

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

On 05/14/2015 06:50 AM, Eric Blake wrote:
> c_type() is designed to be called on both string names and on
> array designations, so 'name' is a bit misleading because it
> operates on more than strings.  Also, no caller ever passes
> an empty string.  Finally, + notation is a bit nicer to read
> than '%s' % value for string concatenation.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---

>      else:
> -        return '%s *%s' % (name, eatspace)
> +        # complex type name
> +        assert isinstance(value, str) and str != ""

D'oh - that should be 'value != ""'.

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 09/16] qapi: Make c_type() consistently convert qapi names

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Continuing the string of cleanups for supporting downstream names
> containing '.', this patch focuses on ensuring c_type() can
> handle a downstream name.  This patch alone does not fix the
> places where generator output should be calling this function
> but was open-coding things instead, but it gets us a step closer.
>
> In particular, the changes to c_list_type() and type_name() mean
> that type_name(FOO) now handles the case when FOO contains '.',
> '-', or is a ticklish identifier other than a builtin (builtins
> are exempted because ['int'] must remain mapped to 'intList' and
> not 'q_intList').  Meanwhile, ['unix'] now maps to 'q_unixList'
> rather than 'unixList', to match the fact that 'unix' is ticklish;
> however, our naming conventions state that complex types should
> start with a capital, so no type name following conventions will
> ever have the 'q_' prepended.
>
> Likewise, changes to c_type() mean that c_type(FOO) properly
> handles an enum or complex type FOO with '.' or '-' in the
> name, or is a ticklish identifier (again, a ticklish identifier
> as a type name violates conventions).
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v4 10/16] qapi: Support downstream enums

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Enhance the testsuite to cover a downstream enum type and enum
> string.  Update the generator to mangle the enum name in the
> appropriate places.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var'

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Use of python's % operator to format strings is fine if there are
> multiple strings or if there is integer formatting going on, but
> when it is just abused for string concatenation, it looks nicer
> to just use the + operator.  This is particularly true when the
> value being substituted is at the front of the format string,
> rather than the tail.

I quite agree in cases such as 

-        discriminator_type_name = '%sKind' % (name)
+        discriminator_type_name = name + 'Kind'

I have doubts in cases such as

-    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
+    return "qmp_marshal_output_" + c_name(name) + "(retval, ret, &local_err);"

I find the old code makes it easier to grasp the result.  Admittedly
subjective.

> Update an error message (and testsuite coverage) while at it, since
> concatenating a non-string object does not always produce legible
> results.

The new expected test output shows improvement.

> Signed-off-by: Eric Blake <eblake@redhat.com>

I'll take 01-15 now, and have a second look at this one later, okay?

Re: [Qemu-devel] [PATCH v4 08/16] qapi: Tidy c_type logic

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/14/2015 06:50 AM, Eric Blake wrote:
>> c_type() is designed to be called on both string names and on
>> array designations, so 'name' is a bit misleading because it
>> operates on more than strings.  Also, no caller ever passes
>> an empty string.  Finally, + notation is a bit nicer to read
>> than '%s' % value for string concatenation.
>> 
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> 
>> ---
>
>>      else:
>> -        return '%s *%s' % (name, eatspace)
>> +        # complex type name
>> +        assert isinstance(value, str) and str != ""
>
> D'oh - that should be 'value != ""'.

You're right!  Missed it in my review...  I'll fix it up on commit.
While there, I'll indulge in a bit of OCD and append () to c_type in the
subject, if you don't mind.

Re: [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var'

[Eric Blake]

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

On 05/14/2015 10:09 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Use of python's % operator to format strings is fine if there are
>> multiple strings or if there is integer formatting going on, but
>> when it is just abused for string concatenation, it looks nicer
>> to just use the + operator.  This is particularly true when the
>> value being substituted is at the front of the format string,
>> rather than the tail.
> 
> I quite agree in cases such as 
> 
> -        discriminator_type_name = '%sKind' % (name)
> +        discriminator_type_name = name + 'Kind'

I could always split this into the obvious cases vs. the questionable
ones, if that helps.

> 
> I have doubts in cases such as
> 
> -    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
> +    return "qmp_marshal_output_" + c_name(name) + "(retval, ret, &local_err);"
> 
> I find the old code makes it easier to grasp the result.  Admittedly
> subjective.

Yeah, that's a judgment call.

> 
>> Update an error message (and testsuite coverage) while at it, since
>> concatenating a non-string object does not always produce legible
>> results.
> 
> The new expected test output shows improvement.

Also might be worth splitting into its own patch, rather than buried in
the noise of cleanups.

> 
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> I'll take 01-15 now, and have a second look at this one later, okay?

Yeah, I kind of figured that might happen. Works for me :)

-- 
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: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 00/16] Fix qapi mangling of downstream names

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 05/14/2015 06:50 AM, Eric Blake wrote:
>> This series makes it possible to use downstream extensions
>> (such as __com.redhat_xyz) and temporary names (such as x-foo)
>> in every position possible in QAPI schemes, with added tests
>> that the generated code still compiles.
>> 
>> There's still some things we could do to the qapi generator,
>> such as normalizing struct member names and C manglings and
>> creating named implicit types up front on the initial parse
>> rather than multiple times in each backend.  But that should
>> wait until existing pending patches have landed, to minimize
>> rebase churn.
>> 
>> v3 was here:
>> https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00519.html
>> [already queued in Markus tree, but not yet in a pull request, so
>> that queue can be ruthlessly rebased]
>> 
>> v4 includes 2 new patches (although 14/14 is somewhat unrelated,
>
> Make that 16/16 as the new patch that could be deferred to later (I was
> looking at the overall size of v3, while writing about v4)
>
>> and could easily be dropped from the series if it needs another
>> spin without holding up the rest of the series), incorporates
>> Markus' suggestions for more comments, and simplifies special-casing
>> of lists of builtin types.

Applied 01-15/16 to my qapi-next branch, left 16/16 for further review /
discussion, thanks!

Re: [Qemu-devel] [PATCH v4 05/16] qapi: Simplify c_enum_const()

[Alberto Garcia]

On Thu 14 May 2015 02:50:51 PM CEST, Eric Blake <eblake@redhat.com> wrote:
> From: Markus Armbruster <armbru@redhat.com>
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Alberto Garcia <berto@igalia.com>

Berto

Re: [Qemu-devel] [PATCH v4 05/16] qapi: Simplify c_enum_const()

[Markus Armbruster]

Alberto Garcia <berto@igalia.com> writes:

> On Thu 14 May 2015 02:50:51 PM CEST, Eric Blake <eblake@redhat.com> wrote:
>> From: Markus Armbruster <armbru@redhat.com>
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> Reviewed-by: Alberto Garcia <berto@igalia.com>

Already in master.  Thanks anyway!