* [Qemu-devel] [PULL 01/11] qapi script: remember explicitly defined enum values
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 02/11] qapi script: add check for duplicated key Luiz Capitulino
` (10 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Later other scripts will need to check the enum values.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 16 +++++++++++-----
| 2 +-
tests/qapi-schema/qapi-schema-test.out | 10 +++++-----
3 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f3c2a20..023930e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -169,7 +169,7 @@ def parse_schema(fp):
for expr in schema.exprs:
if expr.has_key('enum'):
- add_enum(expr['enum'])
+ add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
add_union(expr)
add_enum('%sKind' % expr['union'])
@@ -289,13 +289,19 @@ def find_union(name):
return union
return None
-def add_enum(name):
+def add_enum(name, enum_values = None):
global enum_types
- enum_types.append(name)
+ enum_types.append({"enum_name": name, "enum_values": enum_values})
-def is_enum(name):
+def find_enum(name):
global enum_types
- return (name in 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 c_type(name):
if name == 'str':
--git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
index e3bd904..4ce3dcf 100644
--- a/tests/qapi-schema/comments.out
+++ b/tests/qapi-schema/comments.out
@@ -1,3 +1,3 @@
[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
[]
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 89b53d4..01685d4 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -15,11 +15,11 @@
OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', 'int')]),
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
-['EnumOne',
- 'UserDefUnionKind',
- 'UserDefFlatUnionKind',
- 'UserDefAnonUnionKind',
- 'UserDefNativeListUnionKind']
+[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
+ {'enum_name': 'UserDefUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
+ {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 02/11] qapi script: add check for duplicated key
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 01/11] qapi script: remember explicitly defined enum values Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 03/11] qapi script: remember line number in schema parsing Luiz Capitulino
` (9 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
It is bad that same key was specified twice, especially when a union has
two branches with same condition. This patch can prevent it.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 2 ++
tests/Makefile | 3 ++-
tests/qapi-schema/duplicate-key.err | 1 +
tests/qapi-schema/duplicate-key.exit | 1 +
tests/qapi-schema/duplicate-key.json | 2 ++
tests/qapi-schema/duplicate-key.out | 0
6 files changed, 8 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/duplicate-key.err
create mode 100644 tests/qapi-schema/duplicate-key.exit
create mode 100644 tests/qapi-schema/duplicate-key.json
create mode 100644 tests/qapi-schema/duplicate-key.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 023930e..d0e7934 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -116,6 +116,8 @@ class QAPISchema:
if self.tok != ':':
raise QAPISchemaError(self, 'Expected ":"')
self.accept()
+ if key in expr:
+ raise QAPISchemaError(self, 'Duplicate key "%s"' % key)
expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
diff --git a/tests/Makefile b/tests/Makefile
index b17d41e..dfe06eb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -142,7 +142,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
missing-comma-object.json non-objects.json \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
- unclosed-list.json unclosed-object.json unclosed-string.json)
+ unclosed-list.json unclosed-object.json unclosed-string.json \
+ duplicate-key.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err
new file mode 100644
index 0000000..0801c6a
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.err
@@ -0,0 +1 @@
+<stdin>:2:10: Duplicate key "key"
diff --git a/tests/qapi-schema/duplicate-key.exit b/tests/qapi-schema/duplicate-key.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/duplicate-key.json b/tests/qapi-schema/duplicate-key.json
new file mode 100644
index 0000000..1b55d88
--- /dev/null
+++ b/tests/qapi-schema/duplicate-key.json
@@ -0,0 +1,2 @@
+{ 'key': 'value',
+ 'key': 'value' }
diff --git a/tests/qapi-schema/duplicate-key.out b/tests/qapi-schema/duplicate-key.out
new file mode 100644
index 0000000..e69de29
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 03/11] qapi script: remember line number in schema parsing
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 01/11] qapi script: remember explicitly defined enum values Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 02/11] qapi script: add check for duplicated key Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 04/11] qapi script: check correctness of union Luiz Capitulino
` (8 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Before this patch, 'QAPISchemaError' scans whole input until 'pos'
to get error line number. After this patch, the scan is avoided since
line number is remembered in schema parsing. This patch also benefits
other error report functions, which would be introduced later.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index d0e7934..1954292 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -39,12 +39,10 @@ class QAPISchemaError(Exception):
def __init__(self, schema, msg):
self.fp = schema.fp
self.msg = msg
- self.line = self.col = 1
- for ch in schema.src[0:schema.pos]:
- if ch == '\n':
- self.line += 1
- self.col = 1
- elif ch == '\t':
+ self.col = 1
+ self.line = schema.line
+ for ch in schema.src[schema.line_pos:schema.pos]:
+ if ch == '\t':
self.col = (self.col + 7) % 8 + 1
else:
self.col += 1
@@ -60,6 +58,8 @@ class QAPISchema:
if self.src == '' or self.src[-1] != '\n':
self.src += '\n'
self.cursor = 0
+ self.line = 1
+ self.line_pos = 0
self.exprs = []
self.accept()
@@ -100,6 +100,8 @@ class QAPISchema:
if self.cursor == len(self.src):
self.tok = None
return
+ self.line += 1
+ self.line_pos = self.cursor
elif not self.tok.isspace():
raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 04/11] qapi script: check correctness of union
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (2 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 03/11] qapi script: remember line number in schema parsing Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 05/11] qapi script: code move for generate_enum_name() Luiz Capitulino
` (7 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Since line info is remembered as QAPISchema.line now, this patch
uses it as additional info for every expr in QAPISchema inside qapi.py,
then improves error message with it in checking of exprs.
For common union the patch will check whether base is a valid complex
type if specified. For flat union it will check whether base presents,
whether discriminator is found in base, whether the key of every branch
is correct when discriminator is an enum type.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi.py | 88 +++++++++++++++++++++-
tests/Makefile | 4 +-
.../qapi-schema/flat-union-invalid-branch-key.err | 1 +
.../qapi-schema/flat-union-invalid-branch-key.exit | 1 +
.../qapi-schema/flat-union-invalid-branch-key.json | 17 +++++
.../qapi-schema/flat-union-invalid-branch-key.out | 0
.../flat-union-invalid-discriminator.err | 1 +
.../flat-union-invalid-discriminator.exit | 1 +
.../flat-union-invalid-discriminator.json | 17 +++++
.../flat-union-invalid-discriminator.out | 0
tests/qapi-schema/flat-union-no-base.err | 1 +
tests/qapi-schema/flat-union-no-base.exit | 1 +
tests/qapi-schema/flat-union-no-base.json | 10 +++
tests/qapi-schema/flat-union-no-base.out | 0
tests/qapi-schema/union-invalid-base.err | 1 +
tests/qapi-schema/union-invalid-base.exit | 1 +
tests/qapi-schema/union-invalid-base.json | 10 +++
tests/qapi-schema/union-invalid-base.out | 0
18 files changed, 151 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json
create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out
create mode 100644 tests/qapi-schema/flat-union-no-base.err
create mode 100644 tests/qapi-schema/flat-union-no-base.exit
create mode 100644 tests/qapi-schema/flat-union-no-base.json
create mode 100644 tests/qapi-schema/flat-union-no-base.out
create mode 100644 tests/qapi-schema/union-invalid-base.err
create mode 100644 tests/qapi-schema/union-invalid-base.exit
create mode 100644 tests/qapi-schema/union-invalid-base.json
create mode 100644 tests/qapi-schema/union-invalid-base.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1954292..f1ca5b6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -50,6 +50,15 @@ class QAPISchemaError(Exception):
def __str__(self):
return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
+class QAPIExprError(Exception):
+ def __init__(self, expr_info, msg):
+ self.fp = expr_info['fp']
+ self.line = expr_info['line']
+ self.msg = msg
+
+ def __str__(self):
+ return "%s:%s: %s" % (self.fp.name, self.line, self.msg)
+
class QAPISchema:
def __init__(self, fp):
@@ -64,7 +73,10 @@ class QAPISchema:
self.accept()
while self.tok != None:
- self.exprs.append(self.get_expr(False))
+ expr_info = {'fp': fp, 'line': self.line}
+ expr_elem = {'expr': self.get_expr(False),
+ 'info': expr_info}
+ self.exprs.append(expr_elem)
def accept(self):
while True:
@@ -162,6 +174,71 @@ class QAPISchema:
raise QAPISchemaError(self, 'Expected "{", "[" or string')
return expr
+def find_base_fields(base):
+ base_struct_define = find_struct(base)
+ if not base_struct_define:
+ return None
+ return base_struct_define['data']
+
+def check_union(expr, expr_info):
+ name = expr['union']
+ base = expr.get('base')
+ discriminator = expr.get('discriminator')
+ members = expr['data']
+
+ # If the object has a member 'base', its value must name a complex type.
+ if base:
+ base_fields = find_base_fields(base)
+ if not base_fields:
+ raise QAPIExprError(expr_info,
+ "Base '%s' is not a valid type"
+ % base)
+
+ # If the union object has no member 'discriminator', it's an
+ # ordinary union.
+ if not discriminator:
+ enum_define = None
+
+ # Else if the value of member 'discriminator' is {}, it's an
+ # anonymous union.
+ elif discriminator == {}:
+ enum_define = None
+
+ # Else, it's a flat union.
+ else:
+ # The object must have a member 'base'.
+ if not base:
+ raise QAPIExprError(expr_info,
+ "Flat union '%s' must have a base field"
+ % name)
+ # The value of member 'discriminator' must name a member of the
+ # base type.
+ discriminator_type = base_fields.get(discriminator)
+ if not discriminator_type:
+ raise QAPIExprError(expr_info,
+ "Discriminator '%s' is not a member of base "
+ "type '%s'"
+ % (discriminator, base))
+ enum_define = find_enum(discriminator_type)
+
+ # Check every branch
+ for (key, value) in members.items():
+ # If this named member's value names an enum type, then all members
+ # of 'data' must also be members of the enum type.
+ if enum_define and 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"]))
+ # Todo: add checking for values. Key is checked as above, value can be
+ # also checked here, but we need more functions to handle array case.
+
+def check_exprs(schema):
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
+ if expr.has_key('union'):
+ check_union(expr, expr_elem['info'])
+
def parse_schema(fp):
try:
schema = QAPISchema(fp)
@@ -171,7 +248,8 @@ def parse_schema(fp):
exprs = []
- for expr in schema.exprs:
+ for expr_elem in schema.exprs:
+ expr = expr_elem['expr']
if expr.has_key('enum'):
add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
@@ -181,6 +259,12 @@ def parse_schema(fp):
add_struct(expr)
exprs.append(expr)
+ try:
+ check_exprs(schema)
+ except QAPIExprError, e:
+ print >>sys.stderr, e
+ exit(1)
+
return exprs
def parse_args(typeinfo):
diff --git a/tests/Makefile b/tests/Makefile
index dfe06eb..6ac9889 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -143,7 +143,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
qapi-schema-test.json quoted-structural-chars.json \
trailing-comma-list.json trailing-comma-object.json \
unclosed-list.json unclosed-object.json unclosed-string.json \
- duplicate-key.json)
+ duplicate-key.json union-invalid-base.json flat-union-no-base.json \
+ flat-union-invalid-discriminator.json \
+ flat-union-invalid-branch-key.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err
new file mode 100644
index 0000000..1125caf
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum'
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.exit b/tests/qapi-schema/flat-union-invalid-branch-key.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.json b/tests/qapi-schema/flat-union-invalid-branch-key.json
new file mode 100644
index 0000000..a624282
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-branch-key.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value_wrong': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.out b/tests/qapi-schema/flat-union-invalid-branch-key.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err
new file mode 100644
index 0000000..cad9dbf
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase'
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.exit b/tests/qapi-schema/flat-union-invalid-discriminator.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.json b/tests/qapi-schema/flat-union-invalid-discriminator.json
new file mode 100644
index 0000000..887157e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-invalid-discriminator.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum_wrong',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.out b/tests/qapi-schema/flat-union-invalid-discriminator.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err
new file mode 100644
index 0000000..e2d7443
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.err
@@ -0,0 +1 @@
+<stdin>:7: Flat union 'TestUnion' must have a base field
diff --git a/tests/qapi-schema/flat-union-no-base.exit b/tests/qapi-schema/flat-union-no-base.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-no-base.json b/tests/qapi-schema/flat-union-no-base.json
new file mode 100644
index 0000000..50f2673
--- /dev/null
+++ b/tests/qapi-schema/flat-union-no-base.json
@@ -0,0 +1,10 @@
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'discriminator': 'enum1',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-no-base.out b/tests/qapi-schema/flat-union-no-base.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err
new file mode 100644
index 0000000..dd8e3d1
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.err
@@ -0,0 +1 @@
+<stdin>:7: Base 'TestBaseWrong' is not a valid type
diff --git a/tests/qapi-schema/union-invalid-base.exit b/tests/qapi-schema/union-invalid-base.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-invalid-base.json b/tests/qapi-schema/union-invalid-base.json
new file mode 100644
index 0000000..1fa4930
--- /dev/null
+++ b/tests/qapi-schema/union-invalid-base.json
@@ -0,0 +1,10 @@
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBaseWrong',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/union-invalid-base.out b/tests/qapi-schema/union-invalid-base.out
new file mode 100644
index 0000000..e69de29
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 05/11] qapi script: code move for generate_enum_name()
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (3 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 04/11] qapi script: check correctness of union Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 06/11] qapi script: use same function to generate enum string Luiz Capitulino
` (6 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Later both qapi-types.py and qapi-visit.py need a common function
for enum name generation.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi-types.py | 10 ----------
scripts/qapi.py | 10 ++++++++++
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 2c6e0dc..35ad993 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -127,16 +127,6 @@ const char *%(name)s_lookup[] = {
''')
return ret
-def generate_enum_name(name):
- if name.isupper():
- return c_fun(name, False)
- new_name = ''
- for c in c_fun(name, False):
- if c.isupper():
- new_name += '_'
- new_name += c
- return new_name.lstrip('_').upper()
-
def generate_enum(name, values):
lookup_decl = mcgen('''
extern const char *%(name)s_lookup[];
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f1ca5b6..0de9fe2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -467,3 +467,13 @@ def guardend(name):
''',
name=guardname(name))
+
+def generate_enum_name(name):
+ if name.isupper():
+ return c_fun(name, False)
+ new_name = ''
+ for c in c_fun(name, False):
+ if c.isupper():
+ new_name += '_'
+ new_name += c
+ return new_name.lstrip('_').upper()
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 06/11] qapi script: use same function to generate enum string
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (4 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 05/11] qapi script: code move for generate_enum_name() Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 07/11] qapi script: support enum type as discriminator in union Luiz Capitulino
` (5 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Prior to this patch, qapi-visit.py used custom code to generate enum
names used for handling a qapi union. Fix it to instead reuse common
code, with identical generated results, and allowing future updates to
generation to only need to touch one place.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
scripts/qapi-types.py | 6 +++---
scripts/qapi-visit.py | 19 +++++++++++++------
scripts/qapi.py | 13 +++++++++----
3 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 35ad993..5885bac 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -144,11 +144,11 @@ typedef enum %(name)s
i = 0
for value in enum_values:
+ enum_full_value = generate_enum_full_value(name, value)
enum_decl += mcgen('''
- %(abbrev)s_%(value)s = %(i)d,
+ %(enum_full_value)s = %(i)d,
''',
- abbrev=de_camel_case(name).upper(),
- value=generate_enum_name(value),
+ enum_full_value = enum_full_value,
i=i)
i += 1
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c6de9ae..0baaf60 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -214,18 +214,22 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
''',
name=name)
+ # For anon union, always use the default enum type automatically generated
+ # as "'%sKind' % (name)"
+ disc_type = '%sKind' % (name)
+
for key in members:
assert (members[key] in builtin_types
or find_struct(members[key])
or find_union(members[key])), "Invalid anonymous union member"
+ enum_full_value = generate_enum_full_value(disc_type, key)
ret += mcgen('''
- case %(abbrev)s_KIND_%(enum)s:
+ case %(enum_full_value)s:
visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, name, &err);
break;
''',
- abbrev = de_camel_case(name).upper(),
- enum = c_fun(de_camel_case(key),False).upper(),
+ enum_full_value = enum_full_value,
c_type = type_name(members[key]),
c_name = c_fun(key))
@@ -255,7 +259,10 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
+ # 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)
if base:
base_fields = find_struct(base)['data']
@@ -313,13 +320,13 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
visit_end_implicit_struct(m, &err);
}'''
+ enum_full_value = generate_enum_full_value(disc_type, key)
ret += mcgen('''
- case %(abbrev)s_KIND_%(enum)s:
+ case %(enum_full_value)s:
''' + fmt + '''
break;
''',
- abbrev = de_camel_case(name).upper(),
- enum = c_fun(de_camel_case(key),False).upper(),
+ enum_full_value = enum_full_value,
c_type=type_name(members[key]),
c_name=c_fun(key))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 0de9fe2..eebc8a7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -468,12 +468,17 @@ def guardend(name):
''',
name=guardname(name))
-def generate_enum_name(name):
- if name.isupper():
- return c_fun(name, False)
+def _generate_enum_value_string(value):
+ if value.isupper():
+ return c_fun(value, False)
new_name = ''
- for c in c_fun(name, False):
+ for c in c_fun(value, False):
if c.isupper():
new_name += '_'
new_name += c
return new_name.lstrip('_').upper()
+
+def generate_enum_full_value(enum_name, enum_value):
+ abbrev_string = de_camel_case(enum_name).upper()
+ value_string = _generate_enum_value_string(enum_value)
+ return "%s_%s" % (abbrev_string, value_string)
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 07/11] qapi script: support enum type as discriminator in union
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (5 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 06/11] qapi script: use same function to generate enum string Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 08/11] qapi: convert BlockdevOptions to use enum discriminator Luiz Capitulino
` (4 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type will be really used as the switch case
condition in generated C code, if discriminator is an enum field.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
docs/qapi-code-gen.txt | 8 +++++--
scripts/qapi-types.py | 18 +++++++++++----
scripts/qapi-visit.py | 29 ++++++++++++++++--------
scripts/qapi.py | 27 +++++++++++++++++++++-
tests/Makefile | 2 +-
tests/qapi-schema/flat-union-reverse-define.err | 0
tests/qapi-schema/flat-union-reverse-define.exit | 1 +
tests/qapi-schema/flat-union-reverse-define.json | 17 ++++++++++++++
tests/qapi-schema/flat-union-reverse-define.out | 9 ++++++++
9 files changed, 94 insertions(+), 17 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..a2e7921 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,11 +123,15 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator must always be a string field.
+then no longer generated). The discriminator can be a string field or a
+predefined enum field. If it is a string field, a hidden enum type will be
+generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
+will be done to verify the correctness. It is recommended to use an enum field.
The above example can then be modified as follows:
+ { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
{ 'type': 'BlockdevCommonOptions',
- 'data': { 'driver': 'str', 'readonly': 'bool' } }
+ 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
{ 'union': 'BlockdevOptions',
'base': 'BlockdevCommonOptions',
'discriminator': 'driver',
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5885bac..10864ef 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -201,14 +201,21 @@ def generate_union(expr):
base = expr.get('base')
discriminator = expr.get('discriminator')
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ discriminator_type_name = enum_define['enum_name']
+ else:
+ discriminator_type_name = '%sKind' % (name)
+
ret = mcgen('''
struct %(name)s
{
- %(name)sKind kind;
+ %(discriminator_type_name)s kind;
union {
void *data;
''',
- name=name)
+ name=name,
+ discriminator_type_name=discriminator_type_name)
for key in typeinfo:
ret += mcgen('''
@@ -389,8 +396,11 @@ for expr in exprs:
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_enum('%sKind' % expr['union'], expr['data'].keys())
- fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
+ 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()))
if expr.get('discriminator') == {}:
fdef.write(generate_anon_union_qtypes(expr))
else:
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 0baaf60..45ce3a9 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -259,10 +259,16 @@ def generate_visit_union(expr):
assert not base
return generate_visit_anon_union(name, members)
- # 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)
+ enum_define = discriminator_find_enum_define(expr)
+ if enum_define:
+ # Use the enum type as discriminator
+ ret = ""
+ 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)
if base:
base_fields = find_struct(base)['data']
@@ -298,15 +304,16 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
pop_indent()
if not discriminator:
- desc_type = "type"
+ disc_key = "type"
else:
- desc_type = discriminator
+ disc_key = discriminator
ret += mcgen('''
- visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
+ visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
if (!err) {
switch ((*obj)->kind) {
''',
- name=name, type=desc_type)
+ disc_type = disc_type,
+ disc_key = disc_key)
for key in members:
if not discriminator:
@@ -517,7 +524,11 @@ for expr in exprs:
ret += generate_visit_list(expr['union'], expr['data'])
fdef.write(ret)
- ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
+ enum_define = discriminator_find_enum_define(expr)
+ ret = ""
+ if not enum_define:
+ ret = generate_decl_enum('%sKind' % expr['union'],
+ expr['data'].keys())
ret += generate_declaration(expr['union'], expr['data'])
fdecl.write(ret)
elif expr.has_key('enum'):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index eebc8a7..2b43ad2 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -180,6 +180,25 @@ def find_base_fields(base):
return None
return base_struct_define['data']
+# Return the discriminator enum define if discriminator is specified as an
+# enum type, otherwise return None.
+def discriminator_find_enum_define(expr):
+ base = expr.get('base')
+ discriminator = expr.get('discriminator')
+
+ if not (discriminator and base):
+ return None
+
+ base_fields = find_base_fields(base)
+ if not base_fields:
+ return None
+
+ discriminator_type = base_fields.get(discriminator)
+ if not discriminator_type:
+ return None
+
+ return find_enum(discriminator_type)
+
def check_union(expr, expr_info):
name = expr['union']
base = expr.get('base')
@@ -254,11 +273,17 @@ def parse_schema(fp):
add_enum(expr['enum'], expr['data'])
elif expr.has_key('union'):
add_union(expr)
- add_enum('%sKind' % expr['union'])
elif expr.has_key('type'):
add_struct(expr)
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'])
+
try:
check_exprs(schema)
except QAPIExprError, e:
diff --git a/tests/Makefile b/tests/Makefile
index 6ac9889..88fca31 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,7 +145,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json)
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json
new file mode 100644
index 0000000..9ea7e72
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.json
@@ -0,0 +1,17 @@
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value1': 'TestTypeA',
+ 'value2': 'TestTypeB' } }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum' } }
+
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
new file mode 100644
index 0000000..03c952e
--- /dev/null
+++ b/tests/qapi-schema/flat-union-reverse-define.out
@@ -0,0 +1,9 @@
+[OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))]),
+ OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
+[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']}]
+[OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum')]))]),
+ OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
+ OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 08/11] qapi: convert BlockdevOptions to use enum discriminator
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (6 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 07/11] qapi script: support enum type as discriminator in union Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 09/11] qapi script: do not allow string discriminator Luiz Capitulino
` (3 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
After this patch, hidden enum type BlockdevOptionsKind will not
be generated, and other API can use enum BlockdevDriver.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
qapi-schema.json | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/qapi-schema.json b/qapi-schema.json
index 6c381b7..f4f9439 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4249,6 +4249,18 @@
'*no-flush': 'bool' } }
##
+# @BlockdevDriver
+#
+# Drivers that are supported in block device operations.
+#
+# Since: 2.0
+##
+{ 'enum': 'BlockdevDriver',
+ 'data': [ 'file', 'http', 'https', 'ftp', 'ftps', 'tftp', 'vvfat', 'blkdebug',
+ 'blkverify', 'bochs', 'cloop', 'cow', 'dmg', 'parallels', 'qcow',
+ 'qcow2', 'qed', 'raw', 'vdi', 'vhdx', 'vmdk', 'vpc', 'quorum' ] }
+
+##
# @BlockdevOptionsBase
#
# Options that are available for all block devices, independent of the block
@@ -4272,7 +4284,7 @@
# Since: 1.7
##
{ 'type': 'BlockdevOptionsBase',
- 'data': { 'driver': 'str',
+ 'data': { 'driver': 'BlockdevDriver',
'*id': 'str',
'*node-name': 'str',
'*discard': 'BlockdevDiscardOptions',
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 09/11] qapi script: do not allow string discriminator
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (7 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 08/11] qapi: convert BlockdevOptions to use enum discriminator Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 10/11] qapi script: do not add "_" for every capitalized char in enum Luiz Capitulino
` (2 subsequent siblings)
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Since enum based discriminators provide better type-safety and
ensure that future qapi additions do not forget to adjust dependent
unions, forbid using string as discriminator from now on.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
docs/qapi-code-gen.txt | 5 +----
scripts/qapi.py | 5 +++++
tests/Makefile | 3 ++-
tests/qapi-schema/flat-union-string-discriminator.err | 1 +
tests/qapi-schema/flat-union-string-discriminator.exit | 1 +
tests/qapi-schema/flat-union-string-discriminator.json | 17 +++++++++++++++++
tests/qapi-schema/flat-union-string-discriminator.out | 0
tests/qapi-schema/qapi-schema-test.json | 9 ++++++---
tests/qapi-schema/qapi-schema-test.out | 5 +++--
tests/test-qmp-input-strict.c | 5 ++++-
tests/test-qmp-input-visitor.c | 10 +++++++---
tests/test-qmp-output-visitor.c | 10 ++++++----
12 files changed, 53 insertions(+), 18 deletions(-)
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json
create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a2e7921..d78921f 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -123,10 +123,7 @@ And it looks like this on the wire:
Flat union types avoid the nesting on the wire. They are used whenever a
specific field of the base type is declared as the discriminator ('type' is
-then no longer generated). The discriminator can be a string field or a
-predefined enum field. If it is a string field, a hidden enum type will be
-generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
-will be done to verify the correctness. It is recommended to use an enum field.
+then no longer generated). The discriminator must be of enumeration type.
The above example can then be modified as follows:
{ 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 2b43ad2..bd00d01 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -239,6 +239,11 @@ def check_union(expr, expr_info):
"type '%s'"
% (discriminator, base))
enum_define = find_enum(discriminator_type)
+ # Do not allow string discriminator
+ if not enum_define:
+ raise QAPIExprError(expr_info,
+ "Discriminator '%s' must be of enumeration "
+ "type" % discriminator)
# Check every branch
for (key, value) in members.items():
diff --git a/tests/Makefile b/tests/Makefile
index 88fca31..e146f81 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -145,7 +145,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
unclosed-list.json unclosed-object.json unclosed-string.json \
duplicate-key.json union-invalid-base.json flat-union-no-base.json \
flat-union-invalid-discriminator.json \
- flat-union-invalid-branch-key.json flat-union-reverse-define.json)
+ flat-union-invalid-branch-key.json flat-union-reverse-define.json \
+ flat-union-string-discriminator.json)
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err
new file mode 100644
index 0000000..8748270
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.err
@@ -0,0 +1 @@
+<stdin>:13: Discriminator 'kind' must be of enumeration type
diff --git a/tests/qapi-schema/flat-union-string-discriminator.exit b/tests/qapi-schema/flat-union-string-discriminator.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/flat-union-string-discriminator.json b/tests/qapi-schema/flat-union-string-discriminator.json
new file mode 100644
index 0000000..e966aeb
--- /dev/null
+++ b/tests/qapi-schema/flat-union-string-discriminator.json
@@ -0,0 +1,17 @@
+{ 'enum': 'TestEnum',
+ 'data': [ 'value1', 'value2' ] }
+
+{ 'type': 'TestBase',
+ 'data': { 'enum1': 'TestEnum', 'kind': 'str' } }
+
+{ 'type': 'TestTypeA',
+ 'data': { 'string': 'str' } }
+
+{ 'type': 'TestTypeB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'TestUnion',
+ 'base': 'TestBase',
+ 'discriminator': 'kind',
+ 'data': { 'kind1': 'TestTypeA',
+ 'kind2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-string-discriminator.out b/tests/qapi-schema/flat-union-string-discriminator.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 471ba47..818c06d 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -37,10 +37,13 @@
'base': 'UserDefZero',
'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+{ 'type': 'UserDefUnionBase',
+ 'data': { 'string': 'str', 'enum1': 'EnumOne' } }
+
{ 'union': 'UserDefFlatUnion',
- 'base': 'UserDefOne',
- 'discriminator': 'string',
- 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+ 'base': 'UserDefUnionBase',
+ 'discriminator': 'enum1',
+ 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } }
# FIXME generated struct UserDefFlatUnion has members for direct base
# UserDefOne, but lacks members for indirect base UserDefZero
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 01685d4..6cd03f3 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -7,7 +7,8 @@
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
- OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
+ OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
@@ -17,7 +18,6 @@
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
{'enum_name': 'UserDefUnionKind', 'enum_values': None},
- {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
{'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
{'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
@@ -27,4 +27,5 @@
OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])]
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 64d72f6..38b5e95 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -146,7 +146,10 @@ static void test_validate_union_flat(TestInputVisitorData *data,
Visitor *v;
Error *errp = NULL;
- v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }");
+ v = validate_test_init(data,
+ "{ 'enum1': 'value1', "
+ "'string': 'str', "
+ "'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp);
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 2dffafc..1729667 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -310,14 +310,18 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp;
- v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }");
+ v = visitor_input_test_init(data,
+ "{ 'enum1': 'value1', "
+ "'string': 'str', "
+ "'boolean': true }");
/* TODO when generator bug is fixed, add 'integer': 41 */
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(err == NULL);
- g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A);
+ g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1);
+ g_assert_cmpstr(tmp->string, ==, "str");
/* TODO g_assert_cmpint(tmp->integer, ==, 41); */
- g_assert_cmpint(tmp->a->boolean, ==, true);
+ g_assert_cmpint(tmp->value1->boolean, ==, true);
qapi_free_UserDefFlatUnion(tmp);
}
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 105f4cf..da27971 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -449,10 +449,11 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
Error *err = NULL;
UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
- tmp->kind = USER_DEF_UNION_KIND_A;
- tmp->a = g_malloc0(sizeof(UserDefA));
+ tmp->kind = ENUM_ONE_VALUE1;
+ tmp->string = g_strdup("str");
+ tmp->value1 = g_malloc0(sizeof(UserDefA));
/* TODO when generator bug is fixed: tmp->integer = 41; */
- tmp->a->boolean = true;
+ tmp->value1->boolean = true;
visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
g_assert(err == NULL);
@@ -461,7 +462,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
g_assert(qobject_type(arg) == QTYPE_QDICT);
qdict = qobject_to_qdict(arg);
- g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a");
+ g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1");
+ g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str");
/* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */
g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 10/11] qapi script: do not add "_" for every capitalized char in enum
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (8 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 09/11] qapi script: do not allow string discriminator Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-11 13:41 ` [Qemu-devel] [PULL 11/11] tests: test-qmp-commands: Fix double free Luiz Capitulino
2014-03-12 11:47 ` [Qemu-devel] [PULL for-2.0 00/11] QMP queue Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
From: Wenchao Xia <wenchaoqemu@gmail.com>
Now "enum AIOContext" will generate AIO_CONTEXT instead of A_I_O_CONTEXT,
"X86CPU" will generate X86_CPU instead of X86_C_P_U.
Signed-off-by: Wenchao Xia <wenchaoqemu@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
include/qapi/qmp/qerror.h | 2 +-
scripts/qapi.py | 26 +++++++++++++++++++-------
target-i386/cpu.c | 2 +-
3 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 25193c9..da75abf 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -159,7 +159,7 @@ void qerror_report_err(Error *err);
ERROR_CLASS_GENERIC_ERROR, "Invalid JSON syntax"
#define QERR_KVM_MISSING_CAP \
- ERROR_CLASS_K_V_M_MISSING_CAP, "Using KVM without %s, %s unavailable"
+ ERROR_CLASS_KVM_MISSING_CAP, "Using KVM without %s, %s unavailable"
#define QERR_MIGRATION_ACTIVE \
ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
diff --git a/scripts/qapi.py b/scripts/qapi.py
index bd00d01..b474c39 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -498,17 +498,29 @@ def guardend(name):
''',
name=guardname(name))
-def _generate_enum_value_string(value):
+# 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):
+ c_fun_str = c_fun(value, False)
if value.isupper():
- return c_fun(value, False)
+ return c_fun_str
+
new_name = ''
- for c in c_fun(value, False):
- if c.isupper():
- 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 generate_enum_full_value(enum_name, enum_value):
- abbrev_string = de_camel_case(enum_name).upper()
- value_string = _generate_enum_value_string(enum_value)
+ abbrev_string = _generate_enum_string(enum_name)
+ value_string = _generate_enum_string(enum_value)
return "%s_%s" % (abbrev_string, value_string)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 0e8812a..c83ab0f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -315,7 +315,7 @@ typedef struct X86RegisterInfo32 {
} X86RegisterInfo32;
#define REGISTER(reg) \
- [R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg }
+ [R_##reg] = { .name = #reg, .qapi_enum = X86_CPU_REGISTER32_##reg }
X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = {
REGISTER(EAX),
REGISTER(ECX),
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [Qemu-devel] [PULL 11/11] tests: test-qmp-commands: Fix double free
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (9 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 10/11] qapi script: do not add "_" for every capitalized char in enum Luiz Capitulino
@ 2014-03-11 13:41 ` Luiz Capitulino
2014-03-12 11:47 ` [Qemu-devel] [PULL for-2.0 00/11] QMP queue Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Luiz Capitulino @ 2014-03-11 13:41 UTC (permalink / raw)
To: peter.maydell; +Cc: qemu-devel, anthony
The ret variable is freed twice, but on the second time we actually want
to free ret3 instead. Don't know why this didn't explode.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
tests/test-qmp-commands.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 8e62c2d..554e222 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -141,7 +141,7 @@ static void test_dispatch_cmd_io(void)
ret3 = qobject_to_qint(test_qmp_dispatch(req));
assert(qint_get_int(ret3) == 66);
- QDECREF(ret);
+ QDECREF(ret3);
QDECREF(req);
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [Qemu-devel] [PULL for-2.0 00/11] QMP queue
2014-03-11 13:41 [Qemu-devel] [PULL for-2.0 00/11] QMP queue Luiz Capitulino
` (10 preceding siblings ...)
2014-03-11 13:41 ` [Qemu-devel] [PULL 11/11] tests: test-qmp-commands: Fix double free Luiz Capitulino
@ 2014-03-12 11:47 ` Peter Maydell
11 siblings, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2014-03-12 11:47 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: QEMU Developers, Anthony Liguori
On 11 March 2014 13:41, Luiz Capitulino <lcapitulino@redhat.com> wrote:
> The following changes since commit c57ec3249e9839c7ea2e3789f6e40f9ec1c92f55:
>
> Merge remote-tracking branch 'remotes/kraxel/tags/pull-input-5' into staging (2014-03-11 12:52:08 +0000)
>
> are available in the git repository at:
>
>
> git://repo.or.cz/qemu/qmp-unstable.git queue/qmp
>
> for you to fetch changes up to 2a7a1a56d1e30de07cf7d7636a35bf7706b9500e:
>
> tests: test-qmp-commands: Fix double free (2014-03-11 09:07:42 -0400)
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 13+ messages in thread