[Qemu-devel] [PATCH 00/16] qapi: Miscellaneous fixes and cleanups

[Qemu-devel] [PATCH 00/16] qapi: Miscellaneous fixes and cleanups

[Markus Armbruster]

Output generated for correct input not changed.

Markus Armbruster (16):
  MAINTAINERS: Fix up QAPI and QAPI schema file patterns
  qapi: Drop bogus command from docs
  qapi: Eliminate superfluous QAPISchema attribute input_dir
  qapi: Eliminate superfluous QAPISchema attribute include_hist
  qapi: Improve a couple of confusing variable names
  qapi: Fix file name in error messages for included files
  qapi: Simplify inclusion cycle detection
  qapi: Fix to reject stray 't', 'f' and 'n'
  qapi: Move exprs checking from parse_schema() to check_exprs()
  qapi: Better separate the different kinds of helpers
  tests/qapi-schema: New flat union array branch test case
  qapi: Catch and reject flat union branch of array type
  qapi-types: Don't filter out expressions with 'gen'
  qapi-types: Drop unused members parameters
  qapi-types: Split generate_fwd_builtin() off generate_fwd_struct()
  qapi-types: Bury code dead since commit 6b5abc7

 MAINTAINERS                                    |   4 +
 docs/qapi-code-gen.txt                         |   2 -
 scripts/qapi-types.py                          |  31 +--
 scripts/qapi.py                                | 346 +++++++++++++------------
 tests/Makefile                                 |   3 +-
 tests/qapi-schema/flat-union-array-branch.err  |   1 +
 tests/qapi-schema/flat-union-array-branch.exit |   1 +
 tests/qapi-schema/flat-union-array-branch.json |  12 +
 tests/qapi-schema/flat-union-array-branch.out  |   0
 tests/qapi-schema/include-cycle.err            |   4 +-
 tests/qapi-schema/include-nested-err.err       |   2 +-
 11 files changed, 212 insertions(+), 194 deletions(-)
 create mode 100644 tests/qapi-schema/flat-union-array-branch.err
 create mode 100644 tests/qapi-schema/flat-union-array-branch.exit
 create mode 100644 tests/qapi-schema/flat-union-array-branch.json
 create mode 100644 tests/qapi-schema/flat-union-array-branch.out

-- 
1.9.3

[Qemu-devel] [PATCH 01/16] MAINTAINERS: Fix up QAPI and QAPI schema file patterns

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 MAINTAINERS | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index e728d3a..79f9026 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -946,7 +946,10 @@ M: Markus Armbruster <armbru@redhat.com>
 M: Michael Roth <mdroth@linux.vnet.ibm.com>
 S: Supported
 F: qapi/
+X: qapi/*.json
 F: tests/qapi-schema/
+F: scripts/qapi*
+F: docs/qapi*
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
 QAPI Schema
@@ -954,6 +957,7 @@ M: Eric Blake <eblake@redhat.com>
 M: Markus Armbruster <armbru@redhat.com>
 S: Supported
 F: qapi-schema.json
+F: qapi/*.json
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
 QObject
-- 
1.9.3

[Qemu-devel] [PATCH 02/16] qapi: Drop bogus command from docs

[Markus Armbruster]

Commit 87a560c4 added it in the wrong place.  Commit 59a2c4ce added it
in the right place, but didn't remove it from the wrong place.  Do
that now.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/qapi-code-gen.txt | 2 --
 1 file changed, 2 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 3f0522e..61b5be4 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -680,8 +680,6 @@ Example:
     out:
         error_propagate(errp, err);
     }
-    $ python scripts/qapi-commands.py --output-dir="qapi-generated" \
-    --prefix="example-" example-schema.json
     $ cat qapi-generated/example-qapi-visit.h
 [Uninteresting stuff omitted...]
 
-- 
1.9.3

[Qemu-devel] [PATCH 03/16] qapi: Eliminate superfluous QAPISchema attribute input_dir

[Markus Armbruster]

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

diff --git a/scripts/qapi.py b/scripts/qapi.py
index f96a777..683669e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -109,7 +109,6 @@ class QAPISchema:
         input_fname = os.path.abspath(fp.name)
         if input_relname is None:
             input_relname = fp.name
-        self.input_dir = os.path.dirname(input_fname)
         self.input_file = input_relname
         self.include_hist = include_hist + [(input_relname, input_fname)]
         previously_included.append(input_fname)
@@ -134,7 +133,8 @@ class QAPISchema:
                     raise QAPIExprError(expr_info,
                                         'Expected a file name (string), got: %s'
                                         % include)
-                include_path = os.path.join(self.input_dir, include)
+                include_path = os.path.join(os.path.dirname(input_fname),
+                                            include)
                 for elem in self.include_hist:
                     if include_path == elem[1]:
                         raise QAPIExprError(expr_info, "Inclusion loop for %s"
-- 
1.9.3

[Qemu-devel] [PATCH 04/16] qapi: Eliminate superfluous QAPISchema attribute include_hist

[Markus Armbruster]

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

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 683669e..baa60a5 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -110,7 +110,7 @@ class QAPISchema:
         if input_relname is None:
             input_relname = fp.name
         self.input_file = input_relname
-        self.include_hist = include_hist + [(input_relname, input_fname)]
+        include_hist = include_hist + [(input_relname, input_fname)]
         previously_included.append(input_fname)
         self.parent_info = parent_info
         self.src = fp.read()
@@ -135,7 +135,7 @@ class QAPISchema:
                                         % include)
                 include_path = os.path.join(os.path.dirname(input_fname),
                                             include)
-                for elem in self.include_hist:
+                for elem in include_hist:
                     if include_path == elem[1]:
                         raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                             % include)
@@ -147,7 +147,7 @@ class QAPISchema:
                 except IOError, e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include, self.include_hist,
+                exprs_include = QAPISchema(fobj, include, include_hist,
                                            previously_included, expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
-- 
1.9.3

[Qemu-devel] [PATCH 05/16] qapi: Improve a couple of confusing variable names

[Markus Armbruster]

old name      new name
----------------------------
input_file    fname
input_relname fname
input_fname   abs_fname
include_path  incl_abs_fname
parent_info   incl_info

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 45 +++++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index baa60a5..32f6b95 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -75,7 +75,7 @@ def error_path(parent):
 
 class QAPISchemaError(Exception):
     def __init__(self, schema, msg):
-        self.input_file = schema.input_file
+        self.fname = schema.fname
         self.msg = msg
         self.col = 1
         self.line = schema.line
@@ -84,11 +84,11 @@ class QAPISchemaError(Exception):
                 self.col = (self.col + 7) % 8 + 1
             else:
                 self.col += 1
-        self.info = schema.parent_info
+        self.info = schema.incl_info
 
     def __str__(self):
         return error_path(self.info) + \
-            "%s:%d:%d: %s" % (self.input_file, self.line, self.col, self.msg)
+            "%s:%d:%d: %s" % (self.fname, self.line, self.col, self.msg)
 
 class QAPIExprError(Exception):
     def __init__(self, expr_info, msg):
@@ -101,18 +101,18 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, input_relname=None, include_hist=[],
-                 previously_included=[], parent_info=None):
+    def __init__(self, fp, fname = None, include_hist = [],
+                 previously_included = [], incl_info = None):
         """ include_hist is a stack used to detect inclusion cycles
             previously_included is a global state used to avoid multiple
                                 inclusions of the same file"""
-        input_fname = os.path.abspath(fp.name)
-        if input_relname is None:
-            input_relname = fp.name
-        self.input_file = input_relname
-        include_hist = include_hist + [(input_relname, input_fname)]
-        previously_included.append(input_fname)
-        self.parent_info = parent_info
+        abs_fname = os.path.abspath(fp.name)
+        if fname is None:
+            fname = fp.name
+        self.fname = fname
+        include_hist = include_hist + [(fname, abs_fname)]
+        previously_included.append(abs_fname)
+        self.incl_info = incl_info
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
             self.src += '\n'
@@ -123,7 +123,8 @@ class QAPISchema:
         self.accept()
 
         while self.tok != None:
-            expr_info = {'file': input_relname, 'line': self.line, 'parent': self.parent_info}
+            expr_info = {'file': fname, 'line': self.line,
+                         'parent': self.incl_info}
             expr = self.get_expr(False)
             if isinstance(expr, dict) and "include" in expr:
                 if len(expr) != 1:
@@ -133,17 +134,17 @@ class QAPISchema:
                     raise QAPIExprError(expr_info,
                                         'Expected a file name (string), got: %s'
                                         % include)
-                include_path = os.path.join(os.path.dirname(input_fname),
-                                            include)
+                incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
+                                              include)
                 for elem in include_hist:
-                    if include_path == elem[1]:
+                    if incl_abs_fname == elem[1]:
                         raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                             % include)
                 # skip multiple include of the same file
-                if include_path in previously_included:
+                if incl_abs_fname in previously_included:
                     continue
                 try:
-                    fobj = open(include_path, 'r')
+                    fobj = open(incl_abs_fname, 'r')
                 except IOError, e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
@@ -651,13 +652,13 @@ def check_keys(expr_elem, meta, required, optional=[]):
                                 % (key, meta, name))
 
 
-def parse_schema(input_file):
+def parse_schema(fname):
     global all_names
     exprs = []
 
     # First pass: read entire file into memory
     try:
-        schema = QAPISchema(open(input_file, "r"))
+        schema = QAPISchema(open(fname, "r"))
     except (QAPISchemaError, QAPIExprError), e:
         print >>sys.stderr, e
         exit(1)
@@ -1018,9 +1019,9 @@ def parse_command_line(extra_options = "", extra_long_options = []):
     if len(args) != 1:
         print >>sys.stderr, "%s: need exactly one argument" % sys.argv[0]
         sys.exit(1)
-    input_file = args[0]
+    fname = args[0]
 
-    return (input_file, output_dir, do_c, do_h, prefix, extra_opts)
+    return (fname, output_dir, do_c, do_h, prefix, extra_opts)
 
 def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
                 c_comment, h_comment):
-- 
1.9.3

[Qemu-devel] [PATCH 06/16] qapi: Fix file name in error messages for included files

[Markus Armbruster]

We print the name as it appears in the include expression.  Tools
processing error messages want it relative to the working directory.
Make it so.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                          | 7 +++----
 tests/qapi-schema/include-cycle.err      | 4 ++--
 tests/qapi-schema/include-nested-err.err | 2 +-
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 32f6b95..bde8521 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -101,14 +101,13 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, fname = None, include_hist = [],
+    def __init__(self, fp, include_hist = [],
                  previously_included = [], incl_info = None):
         """ include_hist is a stack used to detect inclusion cycles
             previously_included is a global state used to avoid multiple
                                 inclusions of the same file"""
         abs_fname = os.path.abspath(fp.name)
-        if fname is None:
-            fname = fp.name
+        fname = fp.name
         self.fname = fname
         include_hist = include_hist + [(fname, abs_fname)]
         previously_included.append(abs_fname)
@@ -148,7 +147,7 @@ class QAPISchema:
                 except IOError, e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include, include_hist,
+                exprs_include = QAPISchema(fobj, include_hist,
                                            previously_included, expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
diff --git a/tests/qapi-schema/include-cycle.err b/tests/qapi-schema/include-cycle.err
index 602cf62..bdcd07d 100644
--- a/tests/qapi-schema/include-cycle.err
+++ b/tests/qapi-schema/include-cycle.err
@@ -1,3 +1,3 @@
 In file included from tests/qapi-schema/include-cycle.json:1:
-In file included from include-cycle-b.json:1:
-include-cycle-c.json:1: Inclusion loop for include-cycle.json
+In file included from tests/qapi-schema/include-cycle-b.json:1:
+tests/qapi-schema/include-cycle-c.json:1: Inclusion loop for include-cycle.json
diff --git a/tests/qapi-schema/include-nested-err.err b/tests/qapi-schema/include-nested-err.err
index 1dacbda..1b7b227 100644
--- a/tests/qapi-schema/include-nested-err.err
+++ b/tests/qapi-schema/include-nested-err.err
@@ -1,2 +1,2 @@
 In file included from tests/qapi-schema/include-nested-err.json:1:
-missing-colon.json:1:10: Expected ":"
+tests/qapi-schema/missing-colon.json:1:10: Expected ":"
-- 
1.9.3

[Qemu-devel] [PATCH 07/16] qapi: Simplify inclusion cycle detection

[Markus Armbruster]

We maintain a stack of filenames in include_hist for convenient cycle
detection.

As error_path() demonstrates, the same information is readily
available in the expr_info, so just use that, and drop include_hist.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index bde8521..a24a7e2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -101,15 +101,10 @@ class QAPIExprError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, include_hist = [],
-                 previously_included = [], incl_info = None):
-        """ include_hist is a stack used to detect inclusion cycles
-            previously_included is a global state used to avoid multiple
-                                inclusions of the same file"""
+    def __init__(self, fp, previously_included = [], incl_info = None):
         abs_fname = os.path.abspath(fp.name)
         fname = fp.name
         self.fname = fname
-        include_hist = include_hist + [(fname, abs_fname)]
         previously_included.append(abs_fname)
         self.incl_info = incl_info
         self.src = fp.read()
@@ -135,10 +130,13 @@ class QAPISchema:
                                         % include)
                 incl_abs_fname = os.path.join(os.path.dirname(abs_fname),
                                               include)
-                for elem in include_hist:
-                    if incl_abs_fname == elem[1]:
+                # catch inclusion cycle
+                inf = expr_info
+                while inf:
+                    if incl_abs_fname == os.path.abspath(inf['file']):
                         raise QAPIExprError(expr_info, "Inclusion loop for %s"
                                             % include)
+                    inf = inf['parent']
                 # skip multiple include of the same file
                 if incl_abs_fname in previously_included:
                     continue
@@ -147,8 +145,8 @@ class QAPISchema:
                 except IOError, e:
                     raise QAPIExprError(expr_info,
                                         '%s: %s' % (e.strerror, include))
-                exprs_include = QAPISchema(fobj, include_hist,
-                                           previously_included, expr_info)
+                exprs_include = QAPISchema(fobj, previously_included,
+                                           expr_info)
                 self.exprs.extend(exprs_include.exprs)
             else:
                 expr_elem = {'expr': expr,
-- 
1.9.3

[Qemu-devel] [PATCH 08/16] qapi: Fix to reject stray 't', 'f' and 'n'

[Markus Armbruster]

Screwed up in commit e53188a.

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

diff --git a/scripts/qapi.py b/scripts/qapi.py
index a24a7e2..6faa897 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -217,20 +217,18 @@ class QAPISchema:
                         return
                     else:
                         string += ch
-            elif self.tok in "tfn":
-                val = self.src[self.cursor - 1:]
-                if val.startswith("true"):
-                    self.val = True
-                    self.cursor += 3
-                    return
-                elif val.startswith("false"):
-                    self.val = False
-                    self.cursor += 4
-                    return
-                elif val.startswith("null"):
-                    self.val = None
-                    self.cursor += 3
-                    return
+            elif self.src.startswith("true", self.pos):
+                self.val = True
+                self.cursor += 3
+                return
+            elif self.src.startswith("false", self.pos):
+                self.val = False
+                self.cursor += 4
+                return
+            elif self.src.startswith("null", self.pos):
+                self.val = None
+                self.cursor += 3
+                return
             elif self.tok == '\n':
                 if self.cursor == len(self.src):
                     self.tok = None
-- 
1.9.3

[Qemu-devel] [PATCH 09/16] qapi: Move exprs checking from parse_schema() to check_exprs()

[Markus Armbruster]

To have expression semantic analysis in one place rather than two.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 142 ++++++++++++++++++++++++++------------------------------
 1 file changed, 66 insertions(+), 76 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 6faa897..34a5e8d 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -603,26 +603,6 @@ def check_struct(expr, expr_info):
     if expr.get('base'):
         check_member_clash(expr_info, expr['base'], expr['data'])
 
-def check_exprs(schema):
-    for expr_elem in schema.exprs:
-        expr = expr_elem['expr']
-        info = expr_elem['info']
-
-        if expr.has_key('enum'):
-            check_enum(expr, info)
-        elif expr.has_key('union'):
-            check_union(expr, info)
-        elif expr.has_key('alternate'):
-            check_alternate(expr, info)
-        elif expr.has_key('struct'):
-            check_struct(expr, info)
-        elif expr.has_key('command'):
-            check_command(expr, info)
-        elif expr.has_key('event'):
-            check_event(expr, info)
-        else:
-            assert False, 'unexpected meta type'
-
 def check_keys(expr_elem, meta, required, optional=[]):
     expr = expr_elem['expr']
     info = expr_elem['info']
@@ -646,70 +626,80 @@ def check_keys(expr_elem, meta, required, optional=[]):
                                 "Key '%s' is missing from %s '%s'"
                                 % (key, meta, name))
 
+def check_exprs(exprs):
+    global all_names
+
+    # Learn the types and check for valid expression keys
+    for builtin in builtin_types.keys():
+        all_names[builtin] = 'built-in'
+    for expr_elem in exprs:
+        expr = expr_elem['expr']
+        info = expr_elem['info']
+        if expr.has_key('enum'):
+            check_keys(expr_elem, 'enum', ['data'])
+            add_enum(expr['enum'], info, expr['data'])
+        elif expr.has_key('union'):
+            check_keys(expr_elem, 'union', ['data'],
+                       ['base', 'discriminator'])
+            add_union(expr, info)
+        elif expr.has_key('alternate'):
+            check_keys(expr_elem, 'alternate', ['data'])
+            add_name(expr['alternate'], info, 'alternate')
+        elif expr.has_key('struct'):
+            check_keys(expr_elem, 'struct', ['data'], ['base'])
+            add_struct(expr, info)
+        elif expr.has_key('command'):
+            check_keys(expr_elem, 'command', [],
+                       ['data', 'returns', 'gen', 'success-response'])
+            add_name(expr['command'], info, 'command')
+        elif expr.has_key('event'):
+            check_keys(expr_elem, 'event', [], ['data'])
+            add_name(expr['event'], info, 'event')
+        else:
+            raise QAPIExprError(expr_elem['info'],
+                                "Expression is missing metatype")
+
+    # Try again for hidden UnionKind enum
+    for expr_elem in exprs:
+        expr = expr_elem['expr']
+        if expr.has_key('union'):
+            if not discriminator_find_enum_define(expr):
+                add_enum('%sKind' % expr['union'], expr_elem['info'],
+                         implicit=True)
+        elif expr.has_key('alternate'):
+            add_enum('%sKind' % expr['alternate'], expr_elem['info'],
+                     implicit=True)
+
+    # Validate that exprs make sense
+    for expr_elem in exprs:
+        expr = expr_elem['expr']
+        info = expr_elem['info']
+
+        if expr.has_key('enum'):
+            check_enum(expr, info)
+        elif expr.has_key('union'):
+            check_union(expr, info)
+        elif expr.has_key('alternate'):
+            check_alternate(expr, info)
+        elif expr.has_key('struct'):
+            check_struct(expr, info)
+        elif expr.has_key('command'):
+            check_command(expr, info)
+        elif expr.has_key('event'):
+            check_event(expr, info)
+        else:
+            assert False, 'unexpected meta type'
+
+    return map(lambda expr_elem: expr_elem['expr'], exprs)
 
 def parse_schema(fname):
-    global all_names
-    exprs = []
-
-    # First pass: read entire file into memory
     try:
         schema = QAPISchema(open(fname, "r"))
+        return check_exprs(schema.exprs)
     except (QAPISchemaError, QAPIExprError), e:
         print >>sys.stderr, e
         exit(1)
 
-    try:
-        # Next pass: learn the types and check for valid expression keys. At
-        # this point, top-level 'include' has already been flattened.
-        for builtin in builtin_types.keys():
-            all_names[builtin] = 'built-in'
-        for expr_elem in schema.exprs:
-            expr = expr_elem['expr']
-            info = expr_elem['info']
-            if expr.has_key('enum'):
-                check_keys(expr_elem, 'enum', ['data'])
-                add_enum(expr['enum'], info, expr['data'])
-            elif expr.has_key('union'):
-                check_keys(expr_elem, 'union', ['data'],
-                           ['base', 'discriminator'])
-                add_union(expr, info)
-            elif expr.has_key('alternate'):
-                check_keys(expr_elem, 'alternate', ['data'])
-                add_name(expr['alternate'], info, 'alternate')
-            elif expr.has_key('struct'):
-                check_keys(expr_elem, 'struct', ['data'], ['base'])
-                add_struct(expr, info)
-            elif expr.has_key('command'):
-                check_keys(expr_elem, 'command', [],
-                           ['data', 'returns', 'gen', 'success-response'])
-                add_name(expr['command'], info, 'command')
-            elif expr.has_key('event'):
-                check_keys(expr_elem, 'event', [], ['data'])
-                add_name(expr['event'], info, 'event')
-            else:
-                raise QAPIExprError(expr_elem['info'],
-                                    "Expression is missing metatype")
-            exprs.append(expr)
-
-        # Try again for hidden UnionKind enum
-        for expr_elem in schema.exprs:
-            expr = expr_elem['expr']
-            if expr.has_key('union'):
-                if not discriminator_find_enum_define(expr):
-                    add_enum('%sKind' % expr['union'], expr_elem['info'],
-                             implicit=True)
-            elif expr.has_key('alternate'):
-                add_enum('%sKind' % expr['alternate'], expr_elem['info'],
-                         implicit=True)
-
-        # Final pass - validate that exprs make sense
-        check_exprs(schema)
-    except QAPIExprError, e:
-        print >>sys.stderr, e
-        exit(1)
-
-    return exprs
-
 def parse_args(typeinfo):
     if isinstance(typeinfo, str):
         struct = find_struct(typeinfo)
-- 
1.9.3

[Qemu-devel] [PATCH 10/16] qapi: Better separate the different kinds of helpers

[Markus Armbruster]

Insert comments to separate sections dealing with parsing, semantic
analysis, code generation, and so forth.

Move helpers to their proper section.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py | 128 ++++++++++++++++++++++++++++++++------------------------
 1 file changed, 74 insertions(+), 54 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 34a5e8d..8f23267 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -65,6 +65,10 @@ union_types = []
 events = []
 all_names = {}
 
+#
+# Parsing the schema into expressions
+#
+
 def error_path(parent):
     res = ""
     while parent:
@@ -296,6 +300,10 @@ class QAPISchema:
             raise QAPISchemaError(self, 'Expected "{", "[" or string')
         return expr
 
+#
+# Semantic analysis of schema expressions
+#
+
 def find_base_fields(base):
     base_struct_define = find_struct(base)
     if not base_struct_define:
@@ -356,6 +364,60 @@ def check_name(expr_info, source, name, allow_optional = False,
         raise QAPIExprError(expr_info,
                             "%s uses invalid name '%s'" % (source, name))
 
+def add_name(name, info, meta, implicit = False):
+    global all_names
+    check_name(info, "'%s'" % meta, name)
+    if name in all_names:
+        raise QAPIExprError(info,
+                            "%s '%s' is already defined"
+                            % (all_names[name], name))
+    if not implicit and name[-4:] == 'Kind':
+        raise QAPIExprError(info,
+                            "%s '%s' should not end in 'Kind'"
+                            % (meta, name))
+    all_names[name] = meta
+
+def add_struct(definition, info):
+    global struct_types
+    name = definition['struct']
+    add_name(name, info, 'struct')
+    struct_types.append(definition)
+
+def find_struct(name):
+    global struct_types
+    for struct in struct_types:
+        if struct['struct'] == name:
+            return struct
+    return None
+
+def add_union(definition, info):
+    global union_types
+    name = definition['union']
+    add_name(name, info, 'union')
+    union_types.append(definition)
+
+def find_union(name):
+    global union_types
+    for union in union_types:
+        if union['union'] == name:
+            return union
+    return None
+
+def add_enum(name, info, enum_values = None, implicit = False):
+    global enum_types
+    add_name(name, info, 'enum', implicit)
+    enum_types.append({"enum_name": name, "enum_values": enum_values})
+
+def find_enum(name):
+    global enum_types
+    for enum in enum_types:
+        if enum['enum_name'] == name:
+            return enum
+    return None
+
+def is_enum(name):
+    return find_enum(name) != None
+
 def check_type(expr_info, source, value, allow_array = False,
                allow_dict = False, allow_optional = False,
                allow_star = False, allow_metas = []):
@@ -700,6 +762,10 @@ def parse_schema(fname):
         print >>sys.stderr, e
         exit(1)
 
+#
+# Code generation helpers
+#
+
 def parse_args(typeinfo):
     if isinstance(typeinfo, str):
         struct = find_struct(typeinfo)
@@ -817,60 +883,6 @@ def type_name(value):
         return value
     return c_name(value)
 
-def add_name(name, info, meta, implicit = False):
-    global all_names
-    check_name(info, "'%s'" % meta, name)
-    if name in all_names:
-        raise QAPIExprError(info,
-                            "%s '%s' is already defined"
-                            % (all_names[name], name))
-    if not implicit and name[-4:] == 'Kind':
-        raise QAPIExprError(info,
-                            "%s '%s' should not end in 'Kind'"
-                            % (meta, name))
-    all_names[name] = meta
-
-def add_struct(definition, info):
-    global struct_types
-    name = definition['struct']
-    add_name(name, info, 'struct')
-    struct_types.append(definition)
-
-def find_struct(name):
-    global struct_types
-    for struct in struct_types:
-        if struct['struct'] == name:
-            return struct
-    return None
-
-def add_union(definition, info):
-    global union_types
-    name = definition['union']
-    add_name(name, info, 'union')
-    union_types.append(definition)
-
-def find_union(name):
-    global union_types
-    for union in union_types:
-        if union['union'] == name:
-            return union
-    return None
-
-def add_enum(name, info, enum_values = None, implicit = False):
-    global enum_types
-    add_name(name, info, 'enum', implicit)
-    enum_types.append({"enum_name": name, "enum_values": enum_values})
-
-def find_enum(name):
-    global enum_types
-    for enum in enum_types:
-        if enum['enum_name'] == name:
-            return enum
-    return None
-
-def is_enum(name):
-    return find_enum(name) != None
-
 eatspace = '\033EATSPACE.'
 pointer_suffix = ' *' + eatspace
 
@@ -967,6 +979,10 @@ def guardend(name):
 ''',
                  name=guardname(name))
 
+#
+# Common command line parsing
+#
+
 def parse_command_line(extra_options = "", extra_long_options = []):
 
     try:
@@ -1008,6 +1024,10 @@ def parse_command_line(extra_options = "", extra_long_options = []):
 
     return (fname, output_dir, do_c, do_h, prefix, extra_opts)
 
+#
+# Generate output files with boilerplate
+#
+
 def open_output(output_dir, do_c, do_h, prefix, c_file, h_file,
                 c_comment, h_comment):
     c_file = output_dir + prefix + c_file
-- 
1.9.3

[Qemu-devel] [PATCH 11/16] tests/qapi-schema: New flat union array branch test case

[Markus Armbruster]

The new test demonstrates another generator crash.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/Makefile                                 |  3 ++-
 tests/qapi-schema/flat-union-array-branch.err  | 10 ++++++++++
 tests/qapi-schema/flat-union-array-branch.exit |  1 +
 tests/qapi-schema/flat-union-array-branch.json | 12 ++++++++++++
 tests/qapi-schema/flat-union-array-branch.out  |  0
 5 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100644 tests/qapi-schema/flat-union-array-branch.err
 create mode 100644 tests/qapi-schema/flat-union-array-branch.exit
 create mode 100644 tests/qapi-schema/flat-union-array-branch.json
 create mode 100644 tests/qapi-schema/flat-union-array-branch.out

diff --git a/tests/Makefile b/tests/Makefile
index c5e4744..4de40de 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -237,7 +237,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
 	flat-union-invalid-branch-key.json flat-union-reverse-define.json \
 	flat-union-string-discriminator.json union-base-no-discriminator.json \
 	flat-union-bad-discriminator.json flat-union-bad-base.json \
-	flat-union-base-star.json flat-union-int-branch.json \
+	flat-union-base-star.json \
+	flat-union-array-branch.json flat-union-int-branch.json \
 	flat-union-base-union.json flat-union-branch-clash.json \
 	alternate-nested.json alternate-unknown.json alternate-clash.json \
 	alternate-good.json alternate-base.json alternate-array.json \
diff --git a/tests/qapi-schema/flat-union-array-branch.err b/tests/qapi-schema/flat-union-array-branch.err
new file mode 100644
index 0000000..b45ef43
--- /dev/null
+++ b/tests/qapi-schema/flat-union-array-branch.err
@@ -0,0 +1,10 @@
+Traceback (most recent call last):
+  File "tests/qapi-schema/test-qapi.py", line 19, in <module>
+    exprs = parse_schema(sys.argv[1])
+  File "scripts/qapi.py", line 760, in parse_schema
+    return check_exprs(schema.exprs)
+  File "scripts/qapi.py", line 743, in check_exprs
+    check_union(expr, info)
+  File "scripts/qapi.py", line 586, in check_union
+    assert branch_struct
+AssertionError
diff --git a/tests/qapi-schema/flat-union-array-branch.exit b/tests/qapi-schema/flat-union-array-branch.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-array-branch.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-array-branch.json b/tests/qapi-schema/flat-union-array-branch.json
new file mode 100644
index 0000000..0b98820
--- /dev/null
+++ b/tests/qapi-schema/flat-union-array-branch.json
@@ -0,0 +1,12 @@
+# we require flat union branches to be a struct
+{ 'enum': 'TestEnum',
+  'data': [ 'value1', 'value2' ] }
+{ 'struct': 'Base',
+  'data': { 'enum1': 'TestEnum' } }
+{ 'struct': 'TestTypeB',
+  'data': { 'integer': 'int' } }
+{ 'union': 'TestUnion',
+  'base': 'Base',
+  'discriminator': 'enum1',
+  'data': { 'value1': ['TestTypeB'],
+            'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-array-branch.out b/tests/qapi-schema/flat-union-array-branch.out
new file mode 100644
index 0000000..e69de29
-- 
1.9.3

[Qemu-devel] [PATCH 12/16] qapi: Catch and reject flat union branch of array type

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi.py                               |  2 +-
 tests/qapi-schema/flat-union-array-branch.err | 11 +----------
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8f23267..06d7fc2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -580,7 +580,7 @@ def check_union(expr, expr_info):
         # Each value must name a known type; furthermore, in flat unions,
         # branches must be a struct with no overlapping member names
         check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
-                   value, allow_array=True, allow_metas=allow_metas)
+                   value, allow_array=not base, allow_metas=allow_metas)
         if base:
             branch_struct = find_struct(value)
             assert branch_struct
diff --git a/tests/qapi-schema/flat-union-array-branch.err b/tests/qapi-schema/flat-union-array-branch.err
index b45ef43..8ea91ea 100644
--- a/tests/qapi-schema/flat-union-array-branch.err
+++ b/tests/qapi-schema/flat-union-array-branch.err
@@ -1,10 +1 @@
-Traceback (most recent call last):
-  File "tests/qapi-schema/test-qapi.py", line 19, in <module>
-    exprs = parse_schema(sys.argv[1])
-  File "scripts/qapi.py", line 760, in parse_schema
-    return check_exprs(schema.exprs)
-  File "scripts/qapi.py", line 743, in check_exprs
-    check_union(expr, info)
-  File "scripts/qapi.py", line 586, in check_union
-    assert branch_struct
-AssertionError
+tests/qapi-schema/flat-union-array-branch.json:8: Member 'value1' of union 'TestUnion' cannot be an array
-- 
1.9.3

[Qemu-devel] [PATCH 13/16] qapi-types: Don't filter out expressions with 'gen'

[Markus Armbruster]

Useless, because it can only occur in commands, and we're not dealing
with commands here.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 6bd0b13..86e5ddc 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -329,7 +329,6 @@ fdecl.write(mcgen('''
 '''))
 
 exprs = parse_schema(input_file)
-exprs = filter(lambda expr: not expr.has_key('gen'), exprs)
 
 fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 for typename in builtin_types.keys():
-- 
1.9.3

[Qemu-devel] [PATCH 14/16] qapi-types: Drop unused members parameters

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 86e5ddc..c408542 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -12,7 +12,7 @@
 from ordereddict import OrderedDict
 from qapi import *
 
-def generate_fwd_struct(name, members, builtin_type=False):
+def generate_fwd_struct(name, builtin_type=False):
     if builtin_type:
         return mcgen('''
 
@@ -43,7 +43,7 @@ typedef struct %(name)sList
 ''',
                  name=c_name(name))
 
-def generate_fwd_enum_struct(name, members):
+def generate_fwd_enum_struct(name):
     return mcgen('''
 typedef struct %(name)sList
 {
@@ -332,26 +332,26 @@ exprs = parse_schema(input_file)
 
 fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 for typename in builtin_types.keys():
-    fdecl.write(generate_fwd_struct(typename, None, builtin_type=True))
+    fdecl.write(generate_fwd_struct(typename, builtin_type=True))
 fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 
 for expr in exprs:
     ret = "\n"
     if expr.has_key('struct'):
-        ret += generate_fwd_struct(expr['struct'], expr['data'])
+        ret += generate_fwd_struct(expr['struct'])
     elif expr.has_key('enum'):
         ret += generate_enum(expr['enum'], expr['data']) + "\n"
-        ret += generate_fwd_enum_struct(expr['enum'], expr['data'])
+        ret += generate_fwd_enum_struct(expr['enum'])
         fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
     elif expr.has_key('union'):
-        ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
+        ret += generate_fwd_struct(expr['union']) + "\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'],
                                             expr['data'].keys()))
     elif expr.has_key('alternate'):
-        ret += generate_fwd_struct(expr['alternate'], expr['data']) + "\n"
+        ret += generate_fwd_struct(expr['alternate']) + "\n"
         ret += generate_enum('%sKind' % expr['alternate'], expr['data'].keys())
         fdef.write(generate_enum_lookup('%sKind' % expr['alternate'],
                                         expr['data'].keys()))
-- 
1.9.3

[Qemu-devel] [PATCH 15/16] qapi-types: Split generate_fwd_builtin() off generate_fwd_struct()

[Markus Armbruster]

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

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index c408542..12fb2ef 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -12,9 +12,8 @@
 from ordereddict import OrderedDict
 from qapi import *
 
-def generate_fwd_struct(name, builtin_type=False):
-    if builtin_type:
-        return mcgen('''
+def generate_fwd_builtin(name):
+    return mcgen('''
 
 typedef struct %(name)sList
 {
@@ -25,9 +24,10 @@ typedef struct %(name)sList
     struct %(name)sList *next;
 } %(name)sList;
 ''',
-                     type=c_type(name),
-                     name=name)
+                 type=c_type(name),
+                 name=name)
 
+def generate_fwd_struct(name):
     return mcgen('''
 
 typedef struct %(name)s %(name)s;
@@ -332,7 +332,7 @@ exprs = parse_schema(input_file)
 
 fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 for typename in builtin_types.keys():
-    fdecl.write(generate_fwd_struct(typename, builtin_type=True))
+    fdecl.write(generate_fwd_builtin(typename))
 fdecl.write(guardend("QAPI_TYPES_BUILTIN_STRUCT_DECL"))
 
 for expr in exprs:
-- 
1.9.3

[Qemu-devel] [PATCH 16/16] qapi-types: Bury code dead since commit 6b5abc7

[Markus Armbruster]

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

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 12fb2ef..d28a6b0 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -75,7 +75,6 @@ def generate_struct_fields(members):
 def generate_struct(expr):
 
     structname = expr.get('struct', "")
-    fieldname = expr.get('field', "")
     members = expr['data']
     base = expr.get('base')
 
@@ -98,12 +97,9 @@ struct %(name)s
     char qapi_dummy_field_for_empty_struct;
 ''')
 
-    if len(fieldname):
-        fieldname = " " + fieldname
     ret += mcgen('''
-}%(field)s;
-''',
-            field=fieldname)
+};
+''')
 
     return ret
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH 01/16] MAINTAINERS: Fix up QAPI and QAPI schema file patterns

[Eric Blake]

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

On 06/12/2015 08:50 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  MAINTAINERS | 4 ++++
>  1 file changed, 4 insertions(+)

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

-- 
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 02/16] qapi: Drop bogus command from docs

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Commit 87a560c4 added it in the wrong place.  Commit 59a2c4ce added it
> in the right place, but didn't remove it from the wrong place.  Do
> that now.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/qapi-code-gen.txt | 2 --
>  1 file changed, 2 deletions(-)
> 

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

> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 3f0522e..61b5be4 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -680,8 +680,6 @@ Example:
>      out:
>          error_propagate(errp, err);
>      }
> -    $ python scripts/qapi-commands.py --output-dir="qapi-generated" \
> -    --prefix="example-" example-schema.json
>      $ cat qapi-generated/example-qapi-visit.h
>  [Uninteresting stuff omitted...]

I did a double take there before realizing it was context of the
existing file, and not you hand-editing your patch email :)

-- 
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 03/16] qapi: Eliminate superfluous QAPISchema attribute input_dir

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

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

-- 
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 04/16] qapi: Eliminate superfluous QAPISchema attribute include_hist

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 

Ah, I see - you're changing it from an object member to a local variable
(because it didn't need to persist beyond the one function).

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

-- 
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 05/16] qapi: Improve a couple of confusing variable names

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> old name      new name
> ----------------------------
> input_file    fname
> input_relname fname
> input_fname   abs_fname
> include_path  incl_abs_fname
> parent_info   incl_info

Very useful map.

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 45 +++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 

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

-- 
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 06/16] qapi: Fix file name in error messages for included files

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> We print the name as it appears in the include expression.  Tools
> processing error messages want it relative to the working directory.
> Make it so.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                          | 7 +++----
>  tests/qapi-schema/include-cycle.err      | 4 ++--
>  tests/qapi-schema/include-nested-err.err | 2 +-
>  3 files changed, 6 insertions(+), 7 deletions(-)
> 

> +++ b/tests/qapi-schema/include-cycle.err
> @@ -1,3 +1,3 @@
>  In file included from tests/qapi-schema/include-cycle.json:1:
> -In file included from include-cycle-b.json:1:
> -include-cycle-c.json:1: Inclusion loop for include-cycle.json
> +In file included from tests/qapi-schema/include-cycle-b.json:1:
> +tests/qapi-schema/include-cycle-c.json:1: Inclusion loop for include-cycle.json
> diff --git a/tests/qapi-schema/include-nested-err.err b/tests/qapi-schema/include-nested-err.err
> index 1dacbda..1b7b227 100644
> --- a/tests/qapi-schema/include-nested-err.err
> +++ b/tests/qapi-schema/include-nested-err.err
> @@ -1,2 +1,2 @@
>  In file included from tests/qapi-schema/include-nested-err.json:1:
> -missing-colon.json:1:10: Expected ":"
> +tests/qapi-schema/missing-colon.json:1:10: Expected ":"

Yes, that's nicer to read.  Interesting that deleting a parameter makes
life better!

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

-- 
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 07/16] qapi: Simplify inclusion cycle detection

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> We maintain a stack of filenames in include_hist for convenient cycle
> detection.
> 
> As error_path() demonstrates, the same information is readily
> available in the expr_info, so just use that, and drop include_hist.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
> 

Yet another parameter gone, and the testsuite proves we don't regress in
error message quality.  I'm glad I spent time on the testsuite!

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

-- 
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 08/16] qapi: Fix to reject stray 't', 'f' and 'n'

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Screwed up in commit e53188a.
> 

And partly my fault for taking a patch written in python by someone
else, without being a python guru myself.  :)

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

Is it worth a testsuite enhancement to expose this?

> +            elif self.src.startswith("true", self.pos):
> +                self.val = True
> +                self.cursor += 3
> +                return
> +            elif self.src.startswith("false", self.pos):

We still parse things like bare 'truest' as the token 'true'
concatenated with the nonsense 'st', which is probably not the nicest of
error messages, but the chances of someone mistyping bare words is not
worth making it more robust.

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

-- 
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 09/16] qapi: Move exprs checking from parse_schema() to check_exprs()

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> To have expression semantic analysis in one place rather than two.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 142 ++++++++++++++++++++++++++------------------------------
>  1 file changed, 66 insertions(+), 76 deletions(-)

Again, yay for a good testsuite proving this doesn't mess things up :)


> +
> +    return map(lambda expr_elem: expr_elem['expr'], exprs)

I had to go research what this actually does, but it looks correct.

>  
>  def parse_schema(fname):
> -    global all_names
> -    exprs = []
> -
> -    # First pass: read entire file into memory
>      try:
>          schema = QAPISchema(open(fname, "r"))
> +        return check_exprs(schema.exprs)
>      except (QAPISchemaError, QAPIExprError), e:
>          print >>sys.stderr, e
>          exit(1)

Wow, that's really just a thin wrapper now!

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

-- 
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 10/16] qapi: Better separate the different kinds of helpers

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Insert comments to separate sections dealing with parsing, semantic
> analysis, code generation, and so forth.
> 
> Move helpers to their proper section.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py | 128 ++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 74 insertions(+), 54 deletions(-)
> 

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

-- 
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 11/16] tests/qapi-schema: New flat union array branch test case

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> The new test demonstrates another generator crash.

What, I missed one?

> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/Makefile                                 |  3 ++-
>  tests/qapi-schema/flat-union-array-branch.err  | 10 ++++++++++
>  tests/qapi-schema/flat-union-array-branch.exit |  1 +
>  tests/qapi-schema/flat-union-array-branch.json | 12 ++++++++++++
>  tests/qapi-schema/flat-union-array-branch.out  |  0
>  5 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qapi-schema/flat-union-array-branch.err
>  create mode 100644 tests/qapi-schema/flat-union-array-branch.exit
>  create mode 100644 tests/qapi-schema/flat-union-array-branch.json
>  create mode 100644 tests/qapi-schema/flat-union-array-branch.out
> 

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

-- 
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 12/16] qapi: Catch and reject flat union branch of array type

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi.py                               |  2 +-
>  tests/qapi-schema/flat-union-array-branch.err | 11 +----------
>  2 files changed, 2 insertions(+), 11 deletions(-)
> 

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

-- 
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 13/16] qapi-types: Don't filter out expressions with 'gen'

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Useless, because it can only occur in commands, and we're not dealing
> with commands here.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-types.py | 1 -
>  1 file changed, 1 deletion(-)

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

-- 
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 14/16] qapi-types: Drop unused members parameters

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-types.py | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

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

-- 
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 15/16] qapi-types: Split generate_fwd_builtin() off generate_fwd_struct()

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-types.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 

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

-- 
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 16/16] qapi-types: Bury code dead since commit 6b5abc7

[Eric Blake]

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

On 06/12/2015 08:51 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-types.py | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 

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

-- 
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 08/16] qapi: Fix to reject stray 't', 'f' and 'n'

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 06/12/2015 08:51 AM, Markus Armbruster wrote:
>> Screwed up in commit e53188a.
>> 
>
> And partly my fault for taking a patch written in python by someone
> else, without being a python guru myself.  :)
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  scripts/qapi.py | 26 ++++++++++++--------------
>>  1 file changed, 12 insertions(+), 14 deletions(-)
>> 
>
> Is it worth a testsuite enhancement to expose this?

Four existing tests trigger the same error message:

* bad-type-int.err enum-int-member.err

  Only because we neglect to parse numbers.

* escape-outside-string.err funny-char.err

  These test stray '\' and ';'.

I guess a separate test covering bad literal names would be useful if we
actually do something different there.

>> +            elif self.src.startswith("true", self.pos):
>> +                self.val = True
>> +                self.cursor += 3
>> +                return
>> +            elif self.src.startswith("false", self.pos):
>
> We still parse things like bare 'truest' as the token 'true'
> concatenated with the nonsense 'st', which is probably not the nicest of
> error messages, but the chances of someone mistyping bare words is not
> worth making it more robust.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

What regular expression should the lexer consume for a literal name?

My patch consumes (true|false|null).

For what it's worth, qobject/json-{lexer,parser}.c consume [a-z]+, then
reject anything but true, false, null.  Lexes false0 as two tokens
JSON_KEYWORD false and JSON_INTEGER 0.  Can't find a context where the
parser accepts that pair.  It rejects it with its usual garbage error
message "Invalid JSON syntax".

Re: [Qemu-devel] [PATCH 11/16] tests/qapi-schema: New flat union array branch test case

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 06/12/2015 08:51 AM, Markus Armbruster wrote:
>> The new test demonstrates another generator crash.
>
> What, I missed one?

An obscure one :)

>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  tests/Makefile                                 |  3 ++-
>>  tests/qapi-schema/flat-union-array-branch.err  | 10 ++++++++++
>>  tests/qapi-schema/flat-union-array-branch.exit |  1 +
>>  tests/qapi-schema/flat-union-array-branch.json | 12 ++++++++++++
>>  tests/qapi-schema/flat-union-array-branch.out  |  0
>>  5 files changed, 25 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/qapi-schema/flat-union-array-branch.err
>>  create mode 100644 tests/qapi-schema/flat-union-array-branch.exit
>>  create mode 100644 tests/qapi-schema/flat-union-array-branch.json
>>  create mode 100644 tests/qapi-schema/flat-union-array-branch.out
>> 
>
> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [Qemu-devel] [PATCH 04/16] qapi: Eliminate superfluous QAPISchema attribute include_hist

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 06/12/2015 08:51 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  scripts/qapi.py | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>
> Ah, I see - you're changing it from an object member to a local variable
> (because it didn't need to persist beyond the one function).

Exactly.

I could explain it the commit message, but I think I'll just squash it
into PATCH 07.

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

Thanks!