* [Qemu-devel] [PATCH v6 01/15] qapi: Move empty-enum to compile-time test
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 02/15] qapi: Drop redundant returns-int test Eric Blake
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Rather than just asserting that we can parse an empty enum,
let's also make sure we can compile it, by including it in
qapi-schema-test.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch; could be hoisted earlier alongside subset B v7 3/14
---
tests/Makefile | 1 -
tests/qapi-schema/enum-empty.err | 0
tests/qapi-schema/enum-empty.exit | 1 -
tests/qapi-schema/enum-empty.json | 2 --
tests/qapi-schema/enum-empty.out | 2 --
tests/qapi-schema/qapi-schema-test.json | 6 ++++++
tests/qapi-schema/qapi-schema-test.out | 1 +
7 files changed, 7 insertions(+), 6 deletions(-)
delete mode 100644 tests/qapi-schema/enum-empty.err
delete mode 100644 tests/qapi-schema/enum-empty.exit
delete mode 100644 tests/qapi-schema/enum-empty.json
delete mode 100644 tests/qapi-schema/enum-empty.out
diff --git a/tests/Makefile b/tests/Makefile
index 63542c4..f3626bd 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -261,7 +261,6 @@ qapi-schema += enum-bad-name.json
qapi-schema += enum-bad-prefix.json
qapi-schema += enum-clash-member.json
qapi-schema += enum-dict-member.json
-qapi-schema += enum-empty.json
qapi-schema += enum-int-member.json
qapi-schema += enum-max-member.json
qapi-schema += enum-missing-data.json
diff --git a/tests/qapi-schema/enum-empty.err b/tests/qapi-schema/enum-empty.err
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/enum-empty.exit b/tests/qapi-schema/enum-empty.exit
deleted file mode 100644
index 573541a..0000000
--- a/tests/qapi-schema/enum-empty.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/enum-empty.json b/tests/qapi-schema/enum-empty.json
deleted file mode 100644
index 40d4e85..0000000
--- a/tests/qapi-schema/enum-empty.json
+++ /dev/null
@@ -1,2 +0,0 @@
-# An empty enum, although unusual, is currently acceptable
-{ 'enum': 'MyEnum', 'data': [ ] }
diff --git a/tests/qapi-schema/enum-empty.out b/tests/qapi-schema/enum-empty.out
deleted file mode 100644
index a449d45..0000000
--- a/tests/qapi-schema/enum-empty.out
+++ /dev/null
@@ -1,2 +0,0 @@
-object :empty
-enum MyEnum []
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 020ff2e..758bfc8 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -1,11 +1,17 @@
# *-*- Mode: Python -*-*
+# This file is a stress test of supported qapi constructs that must
+# parse and compile correctly.
+
# for testing enums
{ 'enum': 'EnumOne',
'data': [ 'value1', 'value2', 'value3' ] }
{ 'struct': 'NestedEnumsOne',
'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
+# An empty enum, although unusual, is currently acceptable
+{ 'enum': 'MyEnum', 'data': [ ] }
+
# for testing override of default naming heuristic
{ 'enum': 'QEnumTwo',
'prefix': 'QENUM_TWO',
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index c666481..6b5a048 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -89,6 +89,7 @@ object EventStructOne
object ForceArrays
member unused1: UserDefOneList optional=False
member unused2: UserDefTwoList optional=False
+enum MyEnum []
object NestedEnumsOne
member enum1: EnumOne optional=False
member enum2: EnumOne optional=True
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 02/15] qapi: Drop redundant returns-int test
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 01/15] qapi: Move empty-enum to compile-time test Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 03/15] qapi: Drop redundant flat-union-reverse-define test Eric Blake
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
qapi-schema-test was already testing that we could have a
command returning int, but burned a command name in the whitelist.
Merge the redundant positive test returns-int, and pick a name
that reduces the whitelist size.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch; could be hoisted earlier alongside subset B v7 3/17
---
scripts/qapi.py | 3 ---
tests/Makefile | 1 -
tests/qapi-schema/qapi-schema-test.json | 5 +++--
tests/qapi-schema/qapi-schema-test.out | 10 +++++-----
tests/qapi-schema/returns-int.err | 0
tests/qapi-schema/returns-int.exit | 1 -
tests/qapi-schema/returns-int.json | 3 ---
tests/qapi-schema/returns-int.out | 3 ---
tests/test-qmp-commands.c | 4 ++--
9 files changed, 10 insertions(+), 20 deletions(-)
delete mode 100644 tests/qapi-schema/returns-int.err
delete mode 100644 tests/qapi-schema/returns-int.exit
delete mode 100644 tests/qapi-schema/returns-int.json
delete mode 100644 tests/qapi-schema/returns-int.out
diff --git a/scripts/qapi.py b/scripts/qapi.py
index ae1f935..7cf1db0 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -56,9 +56,6 @@ returns_whitelist = [
'guest-set-vcpus',
'guest-sync',
'guest-sync-delimited',
-
- # From qapi-schema-test:
- 'user_def_cmd3',
]
enum_types = []
diff --git a/tests/Makefile b/tests/Makefile
index f3626bd..e3eb547 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -319,7 +319,6 @@ qapi-schema += redefined-type.json
qapi-schema += returns-alternate.json
qapi-schema += returns-array-bad.json
qapi-schema += returns-dict.json
-qapi-schema += returns-int.json
qapi-schema += returns-unknown.json
qapi-schema += returns-whitelist.json
qapi-schema += struct-base-clash-base.json
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 758bfc8..27ad961 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -108,9 +108,10 @@
{ 'command': 'user_def_cmd2',
'data': {'ud1a': 'UserDefOne', '*ud1b': 'UserDefOne'},
'returns': 'UserDefTwo' }
-{ 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' },
+
+# Returning a non-dictionary requires a name from the whitelist
+{ 'command': 'guest-get-time', 'data': {'a': 'int', '*b': 'int' },
'returns': 'int' }
-# note: command name 'guest-sync' chosen to avoid "cannot use built-in" error
{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
# For testing integer range flattening in opts-visitor. The following schema
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 6b5a048..06d0551 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -17,6 +17,9 @@ object :obj-anyList wrapper
member data: anyList optional=False
object :obj-boolList wrapper
member data: boolList optional=False
+object :obj-guest-get-time arguments
+ member a: int optional=False
+ member b: int optional=True
object :obj-guest-sync arguments
member arg: any optional=False
object :obj-int16List wrapper
@@ -50,9 +53,6 @@ object :obj-user_def_cmd1 arguments
object :obj-user_def_cmd2 arguments
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
-object :obj-user_def_cmd3 arguments
- member a: int optional=False
- member b: int optional=True
alternate AltIntNum
case i: int
case n: number
@@ -187,6 +187,8 @@ object __org.qemu_x-Union2
case __org.qemu_x-value: __org.qemu_x-Struct2
command __org.qemu_x-command :obj-__org.qemu_x-command arguments -> __org.qemu_x-Union1
gen=True success_response=True
+command guest-get-time :obj-guest-get-time arguments -> int
+ gen=True success_response=True
command guest-sync :obj-guest-sync arguments -> any
gen=True success_response=True
command user_def_cmd None -> None
@@ -195,5 +197,3 @@ command user_def_cmd1 :obj-user_def_cmd1 arguments -> None
gen=True success_response=True
command user_def_cmd2 :obj-user_def_cmd2 arguments -> UserDefTwo
gen=True success_response=True
-command user_def_cmd3 :obj-user_def_cmd3 arguments -> int
- gen=True success_response=True
diff --git a/tests/qapi-schema/returns-int.err b/tests/qapi-schema/returns-int.err
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/returns-int.exit b/tests/qapi-schema/returns-int.exit
deleted file mode 100644
index 573541a..0000000
--- a/tests/qapi-schema/returns-int.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/returns-int.json b/tests/qapi-schema/returns-int.json
deleted file mode 100644
index 870ec63..0000000
--- a/tests/qapi-schema/returns-int.json
+++ /dev/null
@@ -1,3 +0,0 @@
-# It is okay (although not extensible) to return a non-dictionary
-# But to make it work, the name must be in a whitelist
-{ 'command': 'guest-get-time', 'returns': 'int' }
diff --git a/tests/qapi-schema/returns-int.out b/tests/qapi-schema/returns-int.out
deleted file mode 100644
index a2da259..0000000
--- a/tests/qapi-schema/returns-int.out
+++ /dev/null
@@ -1,3 +0,0 @@
-object :empty
-command guest-get-time None -> int
- gen=True success_response=True
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 8d5249e..bc59835 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -46,7 +46,7 @@ UserDefTwo *qmp_user_def_cmd2(UserDefOne *ud1a,
return ret;
}
-int64_t qmp_user_def_cmd3(int64_t a, bool has_b, int64_t b, Error **errp)
+int64_t qmp_guest_get_time(int64_t a, bool has_b, int64_t b, Error **errp)
{
return a + (has_b ? b : 0);
}
@@ -160,7 +160,7 @@ static void test_dispatch_cmd_io(void)
qdict_put(args3, "a", qint_from_int(66));
qdict_put(req, "arguments", args3);
- qdict_put(req, "execute", qstring_from_str("user_def_cmd3"));
+ qdict_put(req, "execute", qstring_from_str("guest-get-time"));
ret3 = qobject_to_qint(test_qmp_dispatch(req));
assert(qint_get_int(ret3) == 66);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 03/15] qapi: Drop redundant flat-union-reverse-define test
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 01/15] qapi: Move empty-enum to compile-time test Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 02/15] qapi: Drop redundant returns-int test Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 04/15] qapi: Use generated TestStruct machinery in tests Eric Blake
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
As of commit 8c3f8e77, we test compilation of forward references
for a struct base type (UserDefOne), flat union base type
(UserDefUnionBase), and flat union branch type
(UserDefFlatUnion2). The only remaining forward reference being
tested for parsing in flat-union-reverse-define was a forward
enum declaration. Once we make sure that always compiles,
the smaller parse-only test is redundant and can be deleted.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch; could be hoisted earlier alongside subset B v7 3/14
---
tests/Makefile | 1 -
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 | 13 -------------
tests/qapi-schema/qapi-schema-test.json | 11 +++++++----
6 files changed, 7 insertions(+), 36 deletions(-)
delete mode 100644 tests/qapi-schema/flat-union-reverse-define.err
delete mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
delete mode 100644 tests/qapi-schema/flat-union-reverse-define.json
delete mode 100644 tests/qapi-schema/flat-union-reverse-define.out
diff --git a/tests/Makefile b/tests/Makefile
index e3eb547..791e510 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -287,7 +287,6 @@ qapi-schema += flat-union-invalid-branch-key.json
qapi-schema += flat-union-invalid-discriminator.json
qapi-schema += flat-union-no-base.json
qapi-schema += flat-union-optional-discriminator.json
-qapi-schema += flat-union-reverse-define.json
qapi-schema += flat-union-string-discriminator.json
qapi-schema += funny-char.json
qapi-schema += ident-with-escape.json
diff --git a/tests/qapi-schema/flat-union-reverse-define.err b/tests/qapi-schema/flat-union-reverse-define.err
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/flat-union-reverse-define.exit b/tests/qapi-schema/flat-union-reverse-define.exit
deleted file mode 100644
index 573541a..0000000
--- a/tests/qapi-schema/flat-union-reverse-define.exit
+++ /dev/null
@@ -1 +0,0 @@
-0
diff --git a/tests/qapi-schema/flat-union-reverse-define.json b/tests/qapi-schema/flat-union-reverse-define.json
deleted file mode 100644
index 648bbfe..0000000
--- a/tests/qapi-schema/flat-union-reverse-define.json
+++ /dev/null
@@ -1,17 +0,0 @@
-{ 'union': 'TestUnion',
- 'base': 'TestBase',
- 'discriminator': 'enum1',
- 'data': { 'value1': 'TestTypeA',
- 'value2': 'TestTypeB' } }
-
-{ 'struct': 'TestBase',
- 'data': { 'enum1': 'TestEnum' } }
-
-{ 'enum': 'TestEnum',
- 'data': [ 'value1', 'value2' ] }
-
-{ 'struct': 'TestTypeA',
- 'data': { 'string': 'str' } }
-
-{ 'struct': 'TestTypeB',
- 'data': { 'integer': 'int' } }
diff --git a/tests/qapi-schema/flat-union-reverse-define.out b/tests/qapi-schema/flat-union-reverse-define.out
deleted file mode 100644
index a5a9134..0000000
--- a/tests/qapi-schema/flat-union-reverse-define.out
+++ /dev/null
@@ -1,13 +0,0 @@
-object :empty
-object TestBase
- member enum1: TestEnum optional=False
-enum TestEnum ['value1', 'value2']
-object TestTypeA
- member string: str optional=False
-object TestTypeB
- member integer: int optional=False
-object TestUnion
- base TestBase
- tag enum1
- case value1: TestTypeA
- case value2: TestTypeB
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 27ad961..4e2d7c2 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -4,10 +4,9 @@
# parse and compile correctly.
# for testing enums
-{ 'enum': 'EnumOne',
- 'data': [ 'value1', 'value2', 'value3' ] }
{ 'struct': 'NestedEnumsOne',
- 'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
+ 'data': { 'enum1': 'EnumOne', # Intentional forward reference
+ '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
# An empty enum, although unusual, is currently acceptable
{ 'enum': 'MyEnum', 'data': [ ] }
@@ -20,7 +19,11 @@
# for testing nested structs
{ 'struct': 'UserDefOne',
'base': 'UserDefZero', # intentional forward reference
- 'data': { 'string': 'str', '*enum1': 'EnumOne' } }
+ 'data': { 'string': 'str',
+ '*enum1': 'EnumOne' } } # intentional forward reference
+
+{ 'enum': 'EnumOne',
+ 'data': [ 'value1', 'value2', 'value3' ] }
{ 'struct': 'UserDefZero',
'data': { 'integer': 'int' } }
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 04/15] qapi: Use generated TestStruct machinery in tests
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (2 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 03/15] qapi: Drop redundant flat-union-reverse-define test Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 05/15] qapi: Provide nicer array names in introspection Eric Blake
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Commit d88f5fd and friends first introduced the various test-qmp-*
tests in 2011, with duplicated hand-rolled TestStruct machinery,
to make sure the qapi visitor interface was tested. Later, commit
4f193e3 in 2013 added a .json file for further testing use by the
files, but without consolidating any of the existing hand-rolled
visitors. And with four copies, subtle differences have crept in.
Of course, just because the visitor interface is tested does not
mean it is a sane interface; and future patches will be changing
some of the visitor contracts. Rather than having to duplicate
the cleanup work in each copy of the TestStruct visitor, and keep
each hand-rolled copy in sync with what the generator supplies, we
might as well just test what the generator should give us in the
first place.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
tests/qapi-schema/qapi-schema-test.json | 6 +++-
tests/qapi-schema/qapi-schema-test.out | 5 +++
tests/test-qmp-input-strict.c | 35 --------------------
tests/test-qmp-input-visitor.c | 34 -------------------
tests/test-qmp-output-visitor.c | 58 ---------------------------------
tests/test-visitor-serialization.c | 38 ++-------------------
6 files changed, 12 insertions(+), 164 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 4e2d7c2..f9500b3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -3,6 +3,9 @@
# This file is a stress test of supported qapi constructs that must
# parse and compile correctly.
+{ 'struct': 'TestStruct',
+ 'data': { 'integer': 'int', 'boolean': 'bool', 'string': 'str' } }
+
# for testing enums
{ 'struct': 'NestedEnumsOne',
'data': { 'enum1': 'EnumOne', # Intentional forward reference
@@ -42,7 +45,8 @@
# dummy struct to force generation of array types not otherwise mentioned
{ 'struct': 'ForceArrays',
- 'data': { 'unused1':['UserDefOne'], 'unused2':['UserDefTwo'] } }
+ 'data': { 'unused1':['UserDefOne'], 'unused2':['UserDefTwo'],
+ 'unused3':['TestStruct'] } }
# for testing unions
# Among other things, test that a name collision between branches does
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 06d0551..d452c5b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -89,6 +89,7 @@ object EventStructOne
object ForceArrays
member unused1: UserDefOneList optional=False
member unused2: UserDefTwoList optional=False
+ member unused3: TestStructList optional=False
enum MyEnum []
object NestedEnumsOne
member enum1: EnumOne optional=False
@@ -97,6 +98,10 @@ object NestedEnumsOne
member enum4: EnumOne optional=True
enum QEnumTwo ['value1', 'value2']
prefix QENUM_TWO
+object TestStruct
+ member integer: int optional=False
+ member boolean: bool optional=False
+ member string: str optional=False
object UserDefA
member boolean: bool optional=False
member a_b: int optional=True
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 53a7693..b44184f 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -89,41 +89,6 @@ static Visitor *validate_test_init_raw(TestInputVisitorData *data,
return v;
}
-typedef struct TestStruct
-{
- int64_t integer;
- bool boolean;
- char *string;
-} TestStruct;
-
-static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
- const char *name, Error **errp)
-{
- Error *err = NULL;
-
- visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
- &err);
- if (err) {
- goto out;
- }
-
- visit_type_int(v, &(*obj)->integer, "integer", &err);
- if (err) {
- goto out_end;
- }
- visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
- if (err) {
- goto out_end;
- }
- visit_type_str(v, &(*obj)->string, "string", &err);
-
-out_end:
- error_propagate(errp, err);
- err = NULL;
- visit_end_struct(v, &err);
-out:
- error_propagate(errp, err);
-}
static void test_validate_struct(TestInputVisitorData *data,
const void *unused)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 183a9ec..532a31a 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -185,40 +185,6 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
data->qiv = NULL;
}
-typedef struct TestStruct
-{
- int64_t integer;
- bool boolean;
- char *string;
-} TestStruct;
-
-static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
- const char *name, Error **errp)
-{
- Error *err = NULL;
-
- visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
- &err);
- if (err) {
- goto out;
- }
- visit_type_int(v, &(*obj)->integer, "integer", &err);
- if (err) {
- goto out_end;
- }
- visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
- if (err) {
- goto out_end;
- }
- visit_type_str(v, &(*obj)->string, "string", &err);
-
-out_end:
- error_propagate(errp, err);
- err = NULL;
- visit_end_struct(v, &err);
-out:
- error_propagate(errp, err);
-}
static void test_visitor_in_struct(TestInputVisitorData *data,
const void *unused)
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index c84002e..0ccae04 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -166,41 +166,6 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
}
}
-typedef struct TestStruct
-{
- int64_t integer;
- bool boolean;
- char *string;
-} TestStruct;
-
-static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
- const char *name, Error **errp)
-{
- Error *err = NULL;
-
- visit_start_struct(v, (void **)obj, "TestStruct", name, sizeof(TestStruct),
- &err);
- if (err) {
- goto out;
- }
-
- visit_type_int(v, &(*obj)->integer, "integer", &err);
- if (err) {
- goto out_end;
- }
- visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
- if (err) {
- goto out_end;
- }
- visit_type_str(v, &(*obj)->string, "string", &err);
-
-out_end:
- error_propagate(errp, err);
- err = NULL;
- visit_end_struct(v, &err);
-out:
- error_propagate(errp, err);
-}
static void test_visitor_out_struct(TestOutputVisitorData *data,
const void *unused)
@@ -316,29 +281,6 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
}
}
-typedef struct TestStructList
-{
- union {
- TestStruct *value;
- uint64_t padding;
- };
- struct TestStructList *next;
-} TestStructList;
-
-static void visit_type_TestStructList(Visitor *v, TestStructList **obj,
- const char *name, Error **errp)
-{
- GenericList *i, **head = (GenericList **)obj;
-
- visit_start_list(v, name, errp);
-
- for (*head = i = visit_next_list(v, head, errp); i; i = visit_next_list(v, &i, errp)) {
- TestStructList *native_i = (TestStructList *)i;
- visit_type_TestStruct(v, &native_i->value, NULL, errp);
- }
-
- visit_end_list(v, errp);
-}
static void test_visitor_out_list(TestOutputVisitorData *data,
const void *unused)
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index fa86cae..77ab21f 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -186,40 +186,6 @@ static void visit_primitive_list(Visitor *v, void **native, Error **errp)
}
}
-typedef struct TestStruct
-{
- int64_t integer;
- bool boolean;
- char *string;
-} TestStruct;
-
-static void visit_type_TestStruct(Visitor *v, TestStruct **obj,
- const char *name, Error **errp)
-{
- Error *err = NULL;
-
- visit_start_struct(v, (void **)obj, NULL, name, sizeof(TestStruct), &err);
- if (err) {
- goto out;
- }
-
- visit_type_int(v, &(*obj)->integer, "integer", &err);
- if (err) {
- goto out_end;
- }
- visit_type_bool(v, &(*obj)->boolean, "boolean", &err);
- if (err) {
- goto out_end;
- }
- visit_type_str(v, &(*obj)->string, "string", &err);
-
-out_end:
- error_propagate(errp, err);
- err = NULL;
- visit_end_struct(v, &err);
-out:
- error_propagate(errp, err);
-}
static TestStruct *struct_create(void)
{
@@ -705,7 +671,7 @@ static void test_struct(gconstpointer opaque)
void *serialize_data;
ops->serialize(ts, &serialize_data, visit_struct, &err);
- ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err);
+ ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err);
g_assert(err == NULL);
struct_compare(ts, ts_copy);
@@ -758,7 +724,7 @@ static void test_nested_struct_list(gconstpointer opaque)
ops->serialize(listp, &serialize_data, visit_nested_struct_list, &err);
ops->deserialize((void **)&listp_copy, serialize_data,
- visit_nested_struct_list, &err);
+ visit_nested_struct_list, &err);
g_assert(err == NULL);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 05/15] qapi: Provide nicer array names in introspection
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (3 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 04/15] qapi: Use generated TestStruct machinery in tests Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 06/15] qapi-introspect: Guarantee particular sorting Eric Blake
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
For the sake of humans reading introspection output, it is nice
to have the name of implicit array types be recognizable as
arrays of the underlying type. However, while this patch allows
humans to skip from a command with return type "[123]" straight
to the definition of type "123" without having to first inspect
type "[123]", document that this shortcut should not be taken by
client apps.
This makes the resulting introspection string slightly larger by
default, but slightly smaller when -u is in use (as '[FOO]' is
nicer than 'FOOList' for expressing 'array of FOO').
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: no change
---
docs/qapi-code-gen.txt | 7 +++++--
scripts/qapi-introspect.py | 8 +++++---
2 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 2afab20..6cfc3be 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -658,11 +658,14 @@ Example: the SchemaInfo for BlockRef from section Alternate types
The SchemaInfo for an array type has meta-type "array", and variant
member "element-type", which names the array's element type. Array
-types are implicitly defined.
+types are implicitly defined. For convenience, the array's name may
+resemble the element type; however, clients should examine member
+"element-type" instead of making assumptions based on parsing member
+"name".
Example: the SchemaInfo for ['str']
- { "name": "strList", "meta-type": "array",
+ { "name": "[str]", "meta-type": "array",
"element-type": "str" }
The SchemaInfo for an enumeration type has meta-type "enum" and
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index c0dad66..64f2cd0 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -107,10 +107,12 @@ const char %(c_name)s[] = %(c_string)s;
# characters.
if isinstance(typ, QAPISchemaBuiltinType):
return typ.name
+ if isinstance(typ, QAPISchemaArrayType):
+ return '[' + self._use_type(typ.element_type) + ']'
return self._name(typ.name)
def _gen_json(self, name, mtype, obj):
- if mtype != 'command' and mtype != 'event' and mtype != 'builtin':
+ if mtype not in ('command', 'event', 'builtin', 'array'):
name = self._name(name)
obj['name'] = name
obj['meta-type'] = mtype
@@ -136,8 +138,8 @@ const char %(c_name)s[] = %(c_string)s;
self._gen_json(name, 'enum', {'values': values})
def visit_array_type(self, name, info, element_type):
- self._gen_json(name, 'array',
- {'element-type': self._use_type(element_type)})
+ element = self._use_type(element_type)
+ self._gen_json('[' + element + ']', 'array', {'element-type': element})
def visit_object_type_flat(self, name, info, members, variants):
obj = {'members': [self._gen_member(m) for m in members]}
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 06/15] qapi-introspect: Guarantee particular sorting
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (4 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 05/15] qapi: Provide nicer array names in introspection Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 07/15] qapi: Change alternate layout to use 'type' Eric Blake
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Sorting the values of an enum makes it easier to look up whether
a particular value is present by binary rather than linear search
(probably most visible with QKeyCode, which has grown over
several releases). Additionally, QMP clients need not know which
C value is associated with an enum name, so sorting is an
effective way to hide that non-ABI aspect of qapi.
While we are at it, it is also easy to sort the members and
variants of objects, to allow for a similar binary search, and
equally valid since JSON objects have no specific order in which
keys must appear. There is no trivial or obvious way to sort
the types of an alternate, so that is left unchanged.
However, the overall SchemaInfo array remains unsorted. It might
make sense to sort with 'meta-type' as a primary key and 'name'
as a secondary key, but it is not obvious that this will provide
benefits to end-user clients (we allow mutually recursive types,
so there is no posible topological sorting where a single pass
over the array could resolve all types, and while binary searches
could be made possible by sorting, it would be even more efficient
for clients to read the array into a hashtable for O(1) rather
than O(log n) random-access lookups, at which point pre-sorting is
wasted effort).
Document these conventions, so that clients will know what can
and cannot be relied on.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
TODO: should the documentation mention that the sorting is done
in the C locale? Is there anything required to ensure that python
sorts sanely (ie. that the choice of locale while building
doesn't cause inadvertent sorting differences such as turning on
case-insensitivity)?
v6: no change from v5
---
docs/qapi-code-gen.txt | 21 +++++++++++++++++----
qapi/introspect.json | 22 +++++++++++++++++-----
scripts/qapi-introspect.py | 9 ++++++---
3 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 6cfc3be..a2feb8c 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -512,6 +512,13 @@ query-qmp-schema. QGA currently doesn't support introspection.
query-qmp-schema returns a JSON array of SchemaInfo objects. These
objects together describe the wire ABI, as defined in the QAPI schema.
+There is no specified order to the SchemaInfo objects returned; a
+client must search for a particular name and meta-type throughout the
+entire array to learn more about that name. For now, the name for
+each SchemaInfo is unique thanks to qapi naming conventions; however
+this may be changed in the future (such as allowing a command and an
+event with the same name), so it is important that the client check
+for the desired meta-type.
However, the SchemaInfo can't reflect all the rules and restrictions
that apply to QMP. It's interface introspection (figuring out what's
@@ -592,7 +599,8 @@ any. Each element is a JSON object with members "name" (the member's
name), "type" (the name of its type), and optionally "default". The
member is optional if "default" is present. Currently, "default" can
only have value null. Other values are reserved for future
-extensions.
+extensions. The "members" array is sorted by "name", so that clients
+can use a binary search to see if a particular member is supported.
Example: the SchemaInfo for MyType from section Struct types
@@ -606,7 +614,9 @@ Example: the SchemaInfo for MyType from section Struct types
"variants" is a JSON array describing the object's variant members.
Each element is a JSON object with members "case" (the value of type
tag this element applies to) and "type" (the name of an object type
-that provides the variant members for this type tag value).
+that provides the variant members for this type tag value). The
+"variants" array is sorted by "case", so it appears in the same
+order as the enum type matching "tag".
Example: the SchemaInfo for flat union BlockdevOptions from section
Union types
@@ -647,7 +657,8 @@ Union types
The SchemaInfo for an alternate type has meta-type "alternate", and
variant member "members". "members" is a JSON array. Each element is
a JSON object with member "type", which names a type. Values of the
-alternate type conform to exactly one of its member types.
+alternate type conform to exactly one of its member types. There is
+no guarantee on the order in which "members" will be listed.
Example: the SchemaInfo for BlockRef from section Alternate types
@@ -669,7 +680,9 @@ Example: the SchemaInfo for ['str']
"element-type": "str" }
The SchemaInfo for an enumeration type has meta-type "enum" and
-variant member "values".
+variant member "values". The values are listed in sorted order,
+so clients can use a binary search to see if a particular value
+is present.
Example: the SchemaInfo for MyEnum from section Enumeration types
diff --git a/qapi/introspect.json b/qapi/introspect.json
index cc50dc6..71632af 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -25,6 +25,11 @@
# Returns: array of @SchemaInfo, where each element describes an
# entity in the ABI: command, event, type, ...
#
+# The order of the various SchemaInfo is unspecified. For now, the
+# name of each SchemaInfo is unique regardless of meta-type, but to be
+# safe, clients should check that a given name has the expected
+# meta-type.
+#
# Note: the QAPI schema is also used to help define *internal*
# interfaces, by defining QAPI types. These are not part of the QMP
# wire ABI, and therefore not returned by this command.
@@ -78,7 +83,8 @@
# Commands and events have the name defined in the QAPI schema.
# Unlike command and event names, type names are not part of
# the wire ABI. Consequently, type names are meaningless
-# strings here.
+# strings here. Although all names are currently unique
+# regardless of @meta-type, clients should not rely on this.
#
# All references to other SchemaInfo are by name.
#
@@ -130,7 +136,9 @@
#
# Additional SchemaInfo members for meta-type 'enum'.
#
-# @values: the enumeration type's values.
+# @values: the enumeration type's values. The values are sorted, so
+# clients can use a binary search to see if a particular value
+# is present.
#
# Values of this type are JSON string on the wire.
#
@@ -158,14 +166,18 @@
#
# Additional SchemaInfo members for meta-type 'object'.
#
-# @members: the object type's (non-variant) members.
+# @members: the object type's (non-variant) members. The members are
+# sorted by name, so clients can use a binary search to see
+# if a given member is present.
#
# @tag: #optional the name of the member serving as type tag.
# An element of @members with this name must exist.
#
# @variants: #optional variant members, i.e. additional members that
# depend on the type tag's value. Present exactly when
-# @tag is present.
+# @tag is present. The variants are sorted by case, which
+# means they appear in the same order as the values of the
+# enum type of the @tag.
#
# Values of this type are JSON object on the wire.
#
@@ -219,7 +231,7 @@
#
# Additional SchemaInfo members for meta-type 'alternate'.
#
-# @members: the alternate type's members.
+# @members: the alternate type's members, in no particular order.
# The members' wire encoding is distinct, see
# docs/qapi-code-gen.txt section Alternate types.
#
diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 64f2cd0..6a5a843 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -10,6 +10,7 @@
# See the COPYING file in the top-level directory.
from qapi import *
+from operator import attrgetter
# Caveman's json.dumps() replacement (we're stuck at Python 2.4)
@@ -126,7 +127,8 @@ const char %(c_name)s[] = %(c_string)s;
def _gen_variants(self, tag_name, variants):
return {'tag': tag_name,
- 'variants': [self._gen_variant(v) for v in variants]}
+ 'variants': [self._gen_variant(v) for v in
+ sorted(variants, key=attrgetter('name'))]}
def _gen_variant(self, variant):
return {'case': variant.name, 'type': self._use_type(variant.type)}
@@ -135,14 +137,15 @@ const char %(c_name)s[] = %(c_string)s;
self._gen_json(name, 'builtin', {'json-type': json_type})
def visit_enum_type(self, name, info, values, prefix):
- self._gen_json(name, 'enum', {'values': values})
+ self._gen_json(name, 'enum', {'values': sorted(values)})
def visit_array_type(self, name, info, element_type):
element = self._use_type(element_type)
self._gen_json('[' + element + ']', 'array', {'element-type': element})
def visit_object_type_flat(self, name, info, members, variants):
- obj = {'members': [self._gen_member(m) for m in members]}
+ obj = {'members': [self._gen_member(m) for m in
+ sorted(members, key=attrgetter('name'))]}
if variants:
obj.update(self._gen_variants(variants.tag_member.name,
variants.variants))
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 07/15] qapi: Change alternate layout to use 'type'
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (5 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 06/15] qapi-introspect: Guarantee particular sorting Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 08/15] qapi: Simplify visiting of alternate types Eric Blake
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Prepare to simplify alternate layout by creating a dedicated
subclass for the generated tag type. QMP does not transmit
the tag name, so we can name it whatever we want in C. But
since the tag is closely tied to a qtype_code, this commit
renames the tag from 'kind' to 'type', so the next commit can
then further modify things to use a simpler 'qtype_code type;'
for the tag. As part of this, a new member.c_type() method
will make it possible to express the tag type even if there
is no qapi entity type associated with the tag.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
scripts/qapi-types.py | 2 +-
scripts/qapi-visit.py | 4 ++--
scripts/qapi.py | 20 +++++++++++++++++---
tests/qapi-schema/alternate-clash-type.err | 2 +-
tests/qapi-schema/alternate-clash-type.json | 2 +-
tests/test-qmp-input-visitor.c | 22 +++++++++++-----------
tests/test-qmp-output-visitor.c | 2 +-
7 files changed, 34 insertions(+), 20 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 34ea318..138920c 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -138,7 +138,7 @@ struct %(c_name)s {
ret += mcgen('''
%(c_type)s %(c_name)s;
''',
- c_type=variants.tag_member.type.c_name(),
+ c_type=variants.tag_member.c_type(),
c_name=variants.tag_member.c_name())
# FIXME: What purpose does data serve, besides preventing a union that
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 3f74302..d82e2a3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -179,11 +179,11 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
if (err) {
goto out;
}
- visit_get_next_type(v, (int*) &(*obj)->kind, %(c_name)s_qtypes, name, &err);
+ visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, &err);
if (err) {
goto out_obj;
}
- switch ((*obj)->kind) {
+ switch ((*obj)->type) {
''',
c_name=c_name(name))
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7cf1db0..9a734a8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1015,6 +1015,9 @@ class QAPISchemaObjectTypeMember(object):
def c_name(self):
return c_name(self.name)
+ def c_type(self):
+ return self.type.c_name()
+
def describe(self):
source = self.owner
if source.startswith(':obj-'):
@@ -1119,6 +1122,14 @@ class QAPISchemaAlternateType(QAPISchemaType):
visitor.visit_alternate_type(self.name, self.info, self.variants)
+class QAPISchemaAlternateTypeTag(QAPISchemaObjectTypeMember):
+ def __init__(self, enum_type):
+ QAPISchemaObjectTypeMember.__init__(self, 'type', enum_type, False)
+
+ def _describe(self):
+ return 'implicit tag'
+
+
class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, arg_type, ret_type, gen, success_response):
QAPISchemaEntity.__init__(self, name, info)
@@ -1312,11 +1323,14 @@ class QAPISchema(object):
data = expr['data']
variants = [self._make_variant(key, value)
for (key, value) in data.iteritems()]
- tag_enum = self._make_implicit_tag(name, info, variants)
+ # TODO simplify this to avoid the need for an implicit enum
+ tag_member = QAPISchemaAlternateTypeTag(
+ self._make_implicit_enum_type(name, info,
+ [v.name for v in variants]))
self._def_entity(
QAPISchemaAlternateType(name, info,
QAPISchemaObjectTypeVariants(None,
- tag_enum,
+ tag_member,
variants)))
def _def_command(self, expr, info):
@@ -1640,7 +1654,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
ret += mcgen('''
visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
''',
- c_type=memb.type.c_name(), prefix=prefix, cast=cast,
+ c_type=memb.c_type(), prefix=prefix, cast=cast,
c_name=memb.c_name(), name=memb.name,
errp=errparg)
ret += gen_err_check(skiperr=skiperr)
diff --git a/tests/qapi-schema/alternate-clash-type.err b/tests/qapi-schema/alternate-clash-type.err
index cdd2090..9d922fb 100644
--- a/tests/qapi-schema/alternate-clash-type.err
+++ b/tests/qapi-schema/alternate-clash-type.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-clash-type.json:9: 'kind' (branch of Alt1) collides with 'kind' (implicit tag of Alt1)
+tests/qapi-schema/alternate-clash-type.json:9: 'type' (branch of Alt1) collides with 'type' (implicit tag of Alt1)
diff --git a/tests/qapi-schema/alternate-clash-type.json b/tests/qapi-schema/alternate-clash-type.json
index 629584b..0c7b497 100644
--- a/tests/qapi-schema/alternate-clash-type.json
+++ b/tests/qapi-schema/alternate-clash-type.json
@@ -1,6 +1,6 @@
# Alternate branch 'type'
# Reject this, because we would have a clash in generated C, between the
-# alternate's implicit tag member 'kind' and the branch name 'kind'
+# alternate's implicit tag member 'type' and the branch name 'type'
# within the alternate.
# TODO: Even if alternates are simplified in the future to use a simpler
# 'qtype_code type' tag, rather than a full QAPISchemaObjectTypeMember,
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 532a31a..9540fa0 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -338,14 +338,14 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42");
visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
- g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_I);
+ g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
g_assert_cmpint(tmp->i, ==, 42);
qapi_free_UserDefAlternate(tmp);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "'string'");
visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
- g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_S);
+ g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
g_assert_cmpstr(tmp->s, ==, "string");
qapi_free_UserDefAlternate(tmp);
visitor_input_teardown(data, NULL);
@@ -385,7 +385,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
* parse the same as ans */
v = visitor_input_test_init(data, "42");
visit_type_AltStrNum(v, &asn, NULL, &err);
- /* FIXME g_assert_cmpint(asn->kind, == ALT_STR_NUM_KIND_N); */
+ /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
/* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
g_assert(err);
error_free(err);
@@ -395,28 +395,28 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42");
visit_type_AltNumStr(v, &ans, NULL, &error_abort);
- g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+ g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
g_assert_cmpfloat(ans->n, ==, 42);
qapi_free_AltNumStr(ans);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltStrInt(v, &asi, NULL, &error_abort);
- g_assert_cmpint(asi->kind, ==, ALT_STR_INT_KIND_I);
+ g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
g_assert_cmpint(asi->i, ==, 42);
qapi_free_AltStrInt(asi);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
- g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_I);
+ g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
g_assert_cmpint(ain->i, ==, 42);
qapi_free_AltIntNum(ain);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
- g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_I);
+ g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
g_assert_cmpint(ani->i, ==, 42);
qapi_free_AltNumInt(ani);
visitor_input_teardown(data, NULL);
@@ -433,14 +433,14 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrNum(v, &asn, NULL, &error_abort);
- g_assert_cmpint(asn->kind, ==, ALT_STR_NUM_KIND_N);
+ g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
g_assert_cmpfloat(asn->n, ==, 42.5);
qapi_free_AltStrNum(asn);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumStr(v, &ans, NULL, &error_abort);
- g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
+ g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
g_assert_cmpfloat(ans->n, ==, 42.5);
qapi_free_AltNumStr(ans);
visitor_input_teardown(data, NULL);
@@ -455,14 +455,14 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
- g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_N);
+ g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
g_assert_cmpfloat(ain->n, ==, 42.5);
qapi_free_AltIntNum(ain);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
- g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_N);
+ g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
g_assert_cmpint(ani->n, ==, 42.5);
qapi_free_AltNumInt(ani);
visitor_input_teardown(data, NULL);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 0ccae04..a710557 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -459,7 +459,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
Error *err = NULL;
UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
- tmp->kind = USER_DEF_ALTERNATE_KIND_I;
+ tmp->type = USER_DEF_ALTERNATE_KIND_I;
tmp->i = 42;
visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 08/15] qapi: Simplify visiting of alternate types
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (6 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 07/15] qapi: Change alternate layout to use 'type' Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 09/15] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Previously, working with alternates required two enums, and
some indirection: for type Foo, we created Foo_qtypes[] which
maps each qtype to a member of FooKind_lookup[], then use
FooKind_lookup[] like we do for other union types.
This has a subtle bug: since the values of FooKind_lookup
start at zero, all entries of Foo_qtypes that were not
explicitly initialized map to the same branch of the union as
the first member of the alternate, rather than triggering a
failure in visit_get_next_type(). Fortunately, the bug
seldom bites; the very next thing the input visitor does is
try to parse the incoming JSON with the wrong parser, which
fails; the output visitor is not used with a C struct in that
state, and the dealloc visitor has nothing to clean up (so
there is no leak).
However, it IS observable in one case: the behavior of an
alternate that contains a 'number' member but no 'int' member
differs according to whether the 'number' was first in the
qapi definition, and when the input being parsed is an integer;
this is because the 'number' parser accepts QTYPE_QINT in
addition to the expected QTYPE_QFLOAT. A later patch will worry
about fixing alternates to parse all inputs that a non-alternate
'number' would accept, for now it is still marked FIXME.
This patch fixes the validation bug by deleting the indirection,
and modifying get_next_type() to directly return a qtype code.
There is no longer a need to generate an implicit FooKind array
associated with the alternate type (since the QMP wire format
never uses the stringized counterparts of the C union member
names). Next, the generated visitor is fixed to properly detect
unexpected qtypes in the switch statement. With a bit of work
to the previously-added QAPISchemaAlternateTypeTag, we can wrap
the layout change so that spots in qapi-types that are shared
with other unions still work.
Callers now have to know the QTYPE_* mapping when looking at the
discriminator; but so far, only the testsuite was even using the
C struct of an alternate types. If that gets too confusing, we
could reintroduce FooKind, but initialize it differently than
most generated arrays, as in:
typedef enum FooKind {
FOO_KIND_A = QTYPE_QDICT,
FOO_KIND_B = QTYPE_QINT,
} FooKind;
to create nicer aliases for knowing when to use foo->a or foo->b
when inspecting foo->type. But without a current client, I
didn't see the point of doing it now.
There is a user-visible side effect to this change, but I
consider it to be an improvement. Previously,
the invalid QMP command:
{"execute":"blockdev-add", "arguments":{"options":
{"driver":"raw", "id":"a", "file":true}}}
failed with:
{"error": {"class": "GenericError",
"desc": "Invalid parameter type for 'file', expected: QDict"}}
Now it fails with:
{"error": {"class": "GenericError",
"desc": "Invalid parameter type for 'file', expected: BlockdevRef"}}
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase onto tag_member subclass, testsuite, gen_err_check(),
and info improvements
---
docs/qapi-code-gen.txt | 3 --
include/qapi/visitor-impl.h | 3 +-
include/qapi/visitor.h | 8 +++++-
qapi/qapi-visit-core.c | 4 +--
qapi/qmp-input-visitor.c | 4 +--
scripts/qapi-types.py | 34 -----------------------
scripts/qapi-visit.py | 12 ++++----
scripts/qapi.py | 40 ++++++++++++++++++---------
tests/qapi-schema/alternate-clash-members.err | 2 +-
tests/qapi-schema/alternate-empty.out | 1 -
tests/qapi-schema/qapi-schema-test.out | 8 ------
tests/test-qmp-input-visitor.c | 33 ++++++++++++----------
tests/test-qmp-output-visitor.c | 21 ++++++++++----
13 files changed, 82 insertions(+), 91 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index a2feb8c..0cc432b 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -379,9 +379,6 @@ where each branch of the union names a QAPI type. For example:
'data': { 'definition': 'BlockdevOptions',
'reference': 'str' } }
-Just like for a simple union, an implicit C enum 'NameKind' is created
-to enumerate the branches for the alternate 'Name'.
-
Unlike a union, the discriminator string is never passed on the wire
for the Client JSON Protocol. Instead, the value's JSON type serves
as an implicit discriminator, which in turn means that an alternate
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 8c0ba57..6d95b36 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -32,7 +32,8 @@ struct Visitor
void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
const char *kind, const char *name, Error **errp);
- void (*get_next_type)(Visitor *v, int *kind, const int *qobjects,
+ /* May be NULL; most useful for input visitors. */
+ void (*get_next_type)(Visitor *v, qtype_code *type,
const char *name, Error **errp);
void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index cfc19a6..b765993 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -41,7 +41,13 @@ GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
void visit_end_list(Visitor *v, Error **errp);
void visit_optional(Visitor *v, bool *present, const char *name,
Error **errp);
-void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
+
+/**
+ * Determine the qtype of the item @name in the current object visit.
+ * For input visitors, set *@type to the correct qtype of a qapi
+ * alternate type; for other visitors, leave *@type unchanged.
+ */
+void visit_get_next_type(Visitor *v, qtype_code *type,
const char *name, Error **errp);
void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
const char *kind, const char *name, Error **errp);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 59ed506..3f24daa 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -81,11 +81,11 @@ void visit_optional(Visitor *v, bool *present, const char *name,
}
}
-void visit_get_next_type(Visitor *v, int *obj, const int *qtypes,
+void visit_get_next_type(Visitor *v, qtype_code *type,
const char *name, Error **errp)
{
if (v->get_next_type) {
- v->get_next_type(v, obj, qtypes, name, errp);
+ v->get_next_type(v, type, name, errp);
}
}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5dd9ed5..803ffad 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -208,7 +208,7 @@ static void qmp_input_end_list(Visitor *v, Error **errp)
qmp_input_pop(qiv, errp);
}
-static void qmp_input_get_next_type(Visitor *v, int *kind, const int *qobjects,
+static void qmp_input_get_next_type(Visitor *v, qtype_code *type,
const char *name, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
@@ -218,7 +218,7 @@ static void qmp_input_get_next_type(Visitor *v, int *kind, const int *qobjects,
error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null");
return;
}
- *kind = qobjects[qobject_type(qobj)];
+ *type = qobject_type(qobj);
}
static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 138920c..884eefd 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -87,38 +87,6 @@ struct %(c_name)s {
return ret
-def gen_alternate_qtypes_decl(name):
- return mcgen('''
-
-extern const int %(c_name)s_qtypes[];
-''',
- c_name=c_name(name))
-
-
-def gen_alternate_qtypes(name, variants):
- ret = mcgen('''
-
-const int %(c_name)s_qtypes[QTYPE_MAX] = {
-''',
- c_name=c_name(name))
-
- for var in variants.variants:
- qtype = var.type.alternate_qtype()
- assert qtype
-
- ret += mcgen('''
- [%(qtype)s] = %(enum_const)s,
-''',
- qtype=qtype,
- enum_const=c_enum_const(variants.tag_member.type.name,
- var.name))
-
- ret += mcgen('''
-};
-''')
- return ret
-
-
def gen_union(name, base, variants):
ret = mcgen('''
@@ -266,9 +234,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
def visit_alternate_type(self, name, info, variants):
self._fwdecl += gen_fwd_object_or_array(name)
- self._fwdefn += gen_alternate_qtypes(name, variants)
self.decl += gen_union(name, None, variants)
- self.decl += gen_alternate_qtypes_decl(name)
self._gen_type_cleanup(name)
# If you link code generated from multiple schemata, you want only one
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index d82e2a3..571bfa3 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -179,7 +179,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
if (err) {
goto out;
}
- visit_get_next_type(v, (int*) &(*obj)->type, %(c_name)s_qtypes, name, &err);
+ visit_get_next_type(v, &(*obj)->type, name, &err);
if (err) {
goto out_obj;
}
@@ -193,14 +193,14 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, name, &err);
break;
''',
- case=c_enum_const(variants.tag_member.type.name,
- var.name),
+ case=var.type.alternate_qtype(),
c_type=var.type.c_name(),
c_name=var.c_name())
ret += mcgen('''
default:
- abort();
+ error_setg(&err, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+ "%(name)s");
}
out_obj:
error_propagate(errp, err);
@@ -209,7 +209,8 @@ out_obj:
out:
error_propagate(errp, err);
}
-''')
+''',
+ name=name)
return ret
@@ -411,6 +412,7 @@ fdef.write(mcgen('''
fdecl.write(mcgen('''
#include "qapi/visitor.h"
+#include "qapi/qmp/qerror.h"
#include "%(prefix)sqapi-types.h"
''',
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 9a734a8..430c4bc 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -868,10 +868,7 @@ class QAPISchemaEnumType(QAPISchemaType):
if self.is_implicit():
owner = schema.lookup_type(self.name[:-4])
assert owner
- if isinstance(owner, QAPISchemaAlternateType):
- description = "Alternate '%s' branch" % owner.name
- else:
- description = "Union '%s' branch" % owner.name
+ description = "Union '%s' branch" % owner.name
else:
description = "Enum '%s' value" % self.name
raise QAPIExprError(self.info,
@@ -1060,7 +1057,8 @@ class QAPISchemaObjectTypeVariants(object):
assert self.tag_name == self.tag_member.name
else:
self.tag_member.check(schema, info, members, seen)
- assert isinstance(self.tag_member.type, QAPISchemaEnumType)
+ if not isinstance(self.tag_member, QAPISchemaAlternateTypeTag):
+ assert isinstance(self.tag_member.type, QAPISchemaEnumType)
for v in self.variants:
vseen = dict(seen)
v.check(schema, info, self.tag_member.type, vseen,
@@ -1073,7 +1071,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def check(self, schema, info, tag_type, seen, flat):
QAPISchemaObjectTypeMember.check(self, schema, info, [], seen)
- assert self.name in tag_type.values
+ if tag_type:
+ assert self.name in tag_type.values
if flat:
# For flat unions, check that each QMP member does not
# collide with any non-variant members. No type can
@@ -1113,7 +1112,16 @@ class QAPISchemaAlternateType(QAPISchemaType):
self.variants = variants
def check(self, schema):
- self.variants.check(schema, self.info, [], {})
+ seen = {}
+ self.variants.check(schema, self.info, [], seen)
+ # Ensure no branch names collide with non-variant members
+ for v in self.variants.variants:
+ if v.c_name() in seen:
+ raise QAPIExprError(self.info,
+ "%s collides with %s"
+ % (v.describe(),
+ seen[v.c_name()].describe()))
+ seen[v.c_name()] = v
def json_type(self):
return 'value'
@@ -1123,12 +1131,21 @@ class QAPISchemaAlternateType(QAPISchemaType):
class QAPISchemaAlternateTypeTag(QAPISchemaObjectTypeMember):
- def __init__(self, enum_type):
- QAPISchemaObjectTypeMember.__init__(self, 'type', enum_type, False)
+ def __init__(self):
+ QAPISchemaObjectTypeMember.__init__(self, 'type', '', False)
+
+ def check(self, schema, info, all_members, seen):
+ assert self.owner
+ assert len(seen) == 0
+ all_members.append(self)
+ seen[self.c_name()] = self
def _describe(self):
return 'implicit tag'
+ def c_type(self):
+ return 'qtype_code'
+
class QAPISchemaCommand(QAPISchemaEntity):
def __init__(self, name, info, arg_type, ret_type, gen, success_response):
@@ -1323,10 +1340,7 @@ class QAPISchema(object):
data = expr['data']
variants = [self._make_variant(key, value)
for (key, value) in data.iteritems()]
- # TODO simplify this to avoid the need for an implicit enum
- tag_member = QAPISchemaAlternateTypeTag(
- self._make_implicit_enum_type(name, info,
- [v.name for v in variants]))
+ tag_member = QAPISchemaAlternateTypeTag()
self._def_entity(
QAPISchemaAlternateType(name, info,
QAPISchemaObjectTypeVariants(None,
diff --git a/tests/qapi-schema/alternate-clash-members.err b/tests/qapi-schema/alternate-clash-members.err
index 0adf737..ad8720e 100644
--- a/tests/qapi-schema/alternate-clash-members.err
+++ b/tests/qapi-schema/alternate-clash-members.err
@@ -1 +1 @@
-tests/qapi-schema/alternate-clash-members.json:7: Alternate 'Alt1' branch 'a_b' clashes with 'a-b'
+tests/qapi-schema/alternate-clash-members.json:7: 'a_b' (branch of Alt1) collides with 'a-b' (branch of Alt1)
diff --git a/tests/qapi-schema/alternate-empty.out b/tests/qapi-schema/alternate-empty.out
index 0f153b6..9b010d8 100644
--- a/tests/qapi-schema/alternate-empty.out
+++ b/tests/qapi-schema/alternate-empty.out
@@ -1,4 +1,3 @@
object :empty
alternate Alt
case i: int
-enum AltKind ['i']
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index d452c5b..6826fb6 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -56,27 +56,21 @@ object :obj-user_def_cmd2 arguments
alternate AltIntNum
case i: int
case n: number
-enum AltIntNumKind ['i', 'n']
alternate AltNumInt
case n: number
case i: int
-enum AltNumIntKind ['n', 'i']
alternate AltNumStr
case n: number
case s: str
-enum AltNumStrKind ['n', 's']
alternate AltStrBool
case s: str
case b: bool
-enum AltStrBoolKind ['s', 'b']
alternate AltStrInt
case s: str
case i: int
-enum AltStrIntKind ['s', 'i']
alternate AltStrNum
case s: str
case n: number
-enum AltStrNumKind ['s', 'n']
event EVENT_A None
event EVENT_B None
event EVENT_C :obj-EVENT_C data
@@ -109,7 +103,6 @@ alternate UserDefAlternate
case uda: UserDefA
case s: str
case i: int
-enum UserDefAlternateKind ['uda', 's', 'i']
object UserDefB
member intb: int optional=False
member a-b: bool optional=True
@@ -174,7 +167,6 @@ event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
alternate __org.qemu_x-Alt
case __org.qemu_x-branch: str
case b: __org.qemu_x-Base
-enum __org.qemu_x-AltKind ['__org.qemu_x-branch', 'b']
object __org.qemu_x-Base
member __org.qemu_x-member1: __org.qemu_x-Enum optional=False
enum __org.qemu_x-Enum ['__org.qemu_x-value']
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 9540fa0..c5ed19a 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -338,14 +338,14 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42");
visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
- g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
+ g_assert_cmpint(tmp->type, ==, QTYPE_QINT);
g_assert_cmpint(tmp->i, ==, 42);
qapi_free_UserDefAlternate(tmp);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "'string'");
visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
- g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
+ g_assert_cmpint(tmp->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(tmp->s, ==, "string");
qapi_free_UserDefAlternate(tmp);
visitor_input_teardown(data, NULL);
@@ -381,11 +381,10 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
qapi_free_AltStrBool(asb);
visitor_input_teardown(data, NULL);
- /* FIXME: Order of alternate should not affect semantics; asn should
- * parse the same as ans */
+ /* FIXME: integer should parse as number */
v = visitor_input_test_init(data, "42");
visit_type_AltStrNum(v, &asn, NULL, &err);
- /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
+ /* FIXME g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT); */
/* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
g_assert(err);
error_free(err);
@@ -393,30 +392,34 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
qapi_free_AltStrNum(asn);
visitor_input_teardown(data, NULL);
+ /* FIXME: integer should parse as number */
v = visitor_input_test_init(data, "42");
- visit_type_AltNumStr(v, &ans, NULL, &error_abort);
- g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
- g_assert_cmpfloat(ans->n, ==, 42);
+ visit_type_AltNumStr(v, &ans, NULL, &err);
+ /* FIXME g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT); */
+ /* FIXME g_assert_cmpfloat(ans->n, ==, 42); */
+ g_assert(err);
+ error_free(err);
+ err = NULL;
qapi_free_AltNumStr(ans);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltStrInt(v, &asi, NULL, &error_abort);
- g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
+ g_assert_cmpint(asi->type, ==, QTYPE_QINT);
g_assert_cmpint(asi->i, ==, 42);
qapi_free_AltStrInt(asi);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
- g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
+ g_assert_cmpint(ain->type, ==, QTYPE_QINT);
g_assert_cmpint(ain->i, ==, 42);
qapi_free_AltIntNum(ain);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
- g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
+ g_assert_cmpint(ani->type, ==, QTYPE_QINT);
g_assert_cmpint(ani->i, ==, 42);
qapi_free_AltNumInt(ani);
visitor_input_teardown(data, NULL);
@@ -433,14 +436,14 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrNum(v, &asn, NULL, &error_abort);
- g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
+ g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(asn->n, ==, 42.5);
qapi_free_AltStrNum(asn);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumStr(v, &ans, NULL, &error_abort);
- g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
+ g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ans->n, ==, 42.5);
qapi_free_AltNumStr(ans);
visitor_input_teardown(data, NULL);
@@ -455,14 +458,14 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
- g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
+ g_assert_cmpint(ain->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ain->n, ==, 42.5);
qapi_free_AltIntNum(ain);
visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
- g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
+ g_assert_cmpint(ani->type, ==, QTYPE_QFLOAT);
g_assert_cmpint(ani->n, ==, 42.5);
qapi_free_AltNumInt(ani);
visitor_input_teardown(data, NULL);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index a710557..e72b22c 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -456,20 +456,31 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
const void *unused)
{
QObject *arg;
- Error *err = NULL;
+ UserDefAlternate *tmp;
- UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
- tmp->type = USER_DEF_ALTERNATE_KIND_I;
+ tmp = g_malloc0(sizeof(UserDefAlternate));
+ tmp->type = QTYPE_QINT;
tmp->i = 42;
- visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
arg = qmp_output_get_qobject(data->qov);
g_assert(qobject_type(arg) == QTYPE_QINT);
g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);
qapi_free_UserDefAlternate(tmp);
+
+ tmp = g_malloc0(sizeof(UserDefAlternate));
+ tmp->type = QTYPE_QSTRING;
+ tmp->s = g_strdup("hello");
+
+ visit_type_UserDefAlternate(data->ov, &tmp, NULL, &error_abort);
+ arg = qmp_output_get_qobject(data->qov);
+
+ g_assert(qobject_type(arg) == QTYPE_QSTRING);
+ g_assert_cmpstr(qstring_get_str(qobject_to_qstring(arg)), ==, "hello");
+
+ qapi_free_UserDefAlternate(tmp);
}
static void test_visitor_out_empty(TestOutputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 09/15] qapi: Fix alternates that accept 'number' but not 'int'
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (7 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 08/15] qapi: Simplify visiting of alternate types Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 10/15] qapi: Remove dead visitor code Eric Blake
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
The QMP input visitor allows integral values to be assigned by
promotion to a QTYPE_QFLOAT. However, when parsing an alternate,
we did not take this into account, such that an alternate that
accepts 'number' but not 'int' would reject integral values.
With this patch, we now have the following desirable table:
alternate has case selected for
'int' 'number' QTYPE_QINT QTYPE_QFLOAT
no no error error
no yes 'number' 'number'
yes no 'int' error
yes yes 'int' 'number'
While it is unlikely that we will ever use 'number' in an
alternate other than in the testsuite, it never hurts to be
more precise in what we allow.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase onto earlier testsuite and gen_err_check() improvements
---
include/qapi/visitor-impl.h | 2 +-
include/qapi/visitor.h | 3 ++-
qapi/qapi-visit-core.c | 4 ++--
qapi/qmp-input-visitor.c | 4 ++++
scripts/qapi-visit.py | 9 +++++++--
tests/test-qmp-input-visitor.c | 20 ++++++--------------
6 files changed, 22 insertions(+), 20 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 6d95b36..1d09b7b 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -33,7 +33,7 @@ struct Visitor
void (*type_enum)(Visitor *v, int *obj, const char * const strings[],
const char *kind, const char *name, Error **errp);
/* May be NULL; most useful for input visitors. */
- void (*get_next_type)(Visitor *v, qtype_code *type,
+ void (*get_next_type)(Visitor *v, qtype_code *type, bool promote_int,
const char *name, Error **errp);
void (*type_int)(Visitor *v, int64_t *obj, const char *name, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index b765993..baea594 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -46,8 +46,9 @@ void visit_optional(Visitor *v, bool *present, const char *name,
* Determine the qtype of the item @name in the current object visit.
* For input visitors, set *@type to the correct qtype of a qapi
* alternate type; for other visitors, leave *@type unchanged.
+ * If @promote_int, treat integers as QTYPE_FLOAT.
*/
-void visit_get_next_type(Visitor *v, qtype_code *type,
+void visit_get_next_type(Visitor *v, qtype_code *type, bool promote_int,
const char *name, Error **errp);
void visit_type_enum(Visitor *v, int *obj, const char * const strings[],
const char *kind, const char *name, Error **errp);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 3f24daa..884fe94 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -81,11 +81,11 @@ void visit_optional(Visitor *v, bool *present, const char *name,
}
}
-void visit_get_next_type(Visitor *v, qtype_code *type,
+void visit_get_next_type(Visitor *v, qtype_code *type, bool promote_int,
const char *name, Error **errp)
{
if (v->get_next_type) {
- v->get_next_type(v, type, name, errp);
+ v->get_next_type(v, type, promote_int, name, errp);
}
}
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 803ffad..5310db5 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -209,6 +209,7 @@ static void qmp_input_end_list(Visitor *v, Error **errp)
}
static void qmp_input_get_next_type(Visitor *v, qtype_code *type,
+ bool promote_int,
const char *name, Error **errp)
{
QmpInputVisitor *qiv = to_qiv(v);
@@ -219,6 +220,9 @@ static void qmp_input_get_next_type(Visitor *v, qtype_code *type,
return;
}
*type = qobject_type(qobj);
+ if (promote_int && *type == QTYPE_QINT) {
+ *type = QTYPE_QFLOAT;
+ }
}
static void qmp_input_type_int(Visitor *v, int64_t *obj, const char *name,
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 571bfa3..1ac5350 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -169,6 +169,11 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s *obj, const char *name, Error
def gen_visit_alternate(name, variants):
+ promote_int = 'true'
+ for var in variants.variants:
+ if var.type.alternate_qtype() == 'QTYPE_QINT':
+ promote_int = 'false'
+
ret = mcgen('''
void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
@@ -179,13 +184,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
if (err) {
goto out;
}
- visit_get_next_type(v, &(*obj)->type, name, &err);
+ visit_get_next_type(v, &(*obj)->type, %(promote_int)s, name, &err);
if (err) {
goto out_obj;
}
switch ((*obj)->type) {
''',
- c_name=c_name(name))
+ c_name=c_name(name), promote_int=promote_int)
for var in variants.variants:
ret += mcgen('''
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index c5ed19a..51f1fb7 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -381,25 +381,17 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
qapi_free_AltStrBool(asb);
visitor_input_teardown(data, NULL);
- /* FIXME: integer should parse as number */
v = visitor_input_test_init(data, "42");
- visit_type_AltStrNum(v, &asn, NULL, &err);
- /* FIXME g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT); */
- /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_AltStrNum(v, &asn, NULL, &error_abort);
+ g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
+ g_assert_cmpfloat(asn->n, ==, 42);
qapi_free_AltStrNum(asn);
visitor_input_teardown(data, NULL);
- /* FIXME: integer should parse as number */
v = visitor_input_test_init(data, "42");
- visit_type_AltNumStr(v, &ans, NULL, &err);
- /* FIXME g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT); */
- /* FIXME g_assert_cmpfloat(ans->n, ==, 42); */
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_AltNumStr(v, &ans, NULL, &error_abort);
+ g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
+ g_assert_cmpfloat(ans->n, ==, 42);
qapi_free_AltNumStr(ans);
visitor_input_teardown(data, NULL);
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 10/15] qapi: Remove dead visitor code
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (8 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 09/15] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 11/15] qapi: Plug leaks in test-qmp-* Eric Blake
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Commit cbc95538 removed unused start_handle() and end_handle(),
but forgot got remove their declarations.
Commit 4e27e819 introduced optional visitor callbacks for all
sorts of int types, but except for type_uint64 and type_size,
none of them have ever been supplied (the generic implementation
based on using type_int then bounds-checking works just fine).
In the interest of simplicity, it's easier to make the visitor
callback interface not have to worry about the other sizes.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: no change
---
include/qapi/visitor-impl.h | 19 +++----
include/qapi/visitor.h | 3 -
qapi/qapi-visit-core.c | 131 +++++++++++++++++---------------------------
3 files changed, 58 insertions(+), 95 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 1d09b7b..370935a 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -1,7 +1,7 @@
/*
* Core Definitions for QAPI Visitor implementations
*
- * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012, 2015 Red Hat, Inc.
*
* Author: Paolo Bonizni <pbonzini@redhat.com>
*
@@ -48,18 +48,15 @@ struct Visitor
void (*optional)(Visitor *v, bool *present, const char *name,
Error **errp);
- void (*type_uint8)(Visitor *v, uint8_t *obj, const char *name, Error **errp);
- void (*type_uint16)(Visitor *v, uint16_t *obj, const char *name, Error **errp);
- void (*type_uint32)(Visitor *v, uint32_t *obj, const char *name, Error **errp);
- void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
- void (*type_int8)(Visitor *v, int8_t *obj, const char *name, Error **errp);
- void (*type_int16)(Visitor *v, int16_t *obj, const char *name, Error **errp);
- void (*type_int32)(Visitor *v, int32_t *obj, const char *name, Error **errp);
- void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
- /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
- void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
bool (*start_union)(Visitor *v, bool data_present, Error **errp);
void (*end_union)(Visitor *v, bool data_present, Error **errp);
+
+ /* Only required to visit uint64 differently than (*type_int)(). */
+ void (*type_uint64)(Visitor *v, uint64_t *obj, const char *name,
+ Error **errp);
+ /* Only required to visit sizes differently than (*type_uint64)(). */
+ void (*type_size)(Visitor *v, uint64_t *obj, const char *name,
+ Error **errp);
};
void input_type_enum(Visitor *v, int *obj, const char * const strings[],
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index baea594..67ddd83 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -27,9 +27,6 @@ typedef struct GenericList
struct GenericList *next;
} GenericList;
-void visit_start_handle(Visitor *v, void **obj, const char *kind,
- const char *name, Error **errp);
-void visit_end_handle(Visitor *v, Error **errp);
void visit_start_struct(Visitor *v, void **obj, const char *kind,
const char *name, size_t size, Error **errp);
void visit_end_struct(Visitor *v, Error **errp);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 884fe94..cbf7780 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -104,57 +104,48 @@ void visit_type_uint8(Visitor *v, uint8_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (v->type_uint8) {
- v->type_uint8(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT8_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "uint8_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT8_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "uint8_t");
+ return;
}
+ *obj = value;
}
-void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name, Error **errp)
+void visit_type_uint16(Visitor *v, uint16_t *obj, const char *name,
+ Error **errp)
{
int64_t value;
- if (v->type_uint16) {
- v->type_uint16(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT16_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "uint16_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT16_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "uint16_t");
+ return;
}
+ *obj = value;
}
-void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name, Error **errp)
+void visit_type_uint32(Visitor *v, uint32_t *obj, const char *name,
+ Error **errp)
{
int64_t value;
- if (v->type_uint32) {
- v->type_uint32(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < 0 || value > UINT32_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "uint32_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < 0 || value > UINT32_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "uint32_t");
+ return;
}
+ *obj = value;
}
-void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp)
+void visit_type_uint64(Visitor *v, uint64_t *obj, const char *name,
+ Error **errp)
{
int64_t value;
@@ -171,77 +162,55 @@ void visit_type_int8(Visitor *v, int8_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (v->type_int8) {
- v->type_int8(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT8_MIN || value > INT8_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "int8_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT8_MIN || value > INT8_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "int8_t");
+ return;
}
+ *obj = value;
}
void visit_type_int16(Visitor *v, int16_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (v->type_int16) {
- v->type_int16(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT16_MIN || value > INT16_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "int16_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT16_MIN || value > INT16_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "int16_t");
+ return;
}
+ *obj = value;
}
void visit_type_int32(Visitor *v, int32_t *obj, const char *name, Error **errp)
{
int64_t value;
- if (v->type_int32) {
- v->type_int32(v, obj, name, errp);
- } else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- if (value < INT32_MIN || value > INT32_MAX) {
- error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
- name ? name : "null", "int32_t");
- return;
- }
- *obj = value;
+ value = *obj;
+ v->type_int(v, &value, name, errp);
+ if (value < INT32_MIN || value > INT32_MAX) {
+ error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+ name ? name : "null", "int32_t");
+ return;
}
+ *obj = value;
}
void visit_type_int64(Visitor *v, int64_t *obj, const char *name, Error **errp)
{
- if (v->type_int64) {
- v->type_int64(v, obj, name, errp);
- } else {
- v->type_int(v, obj, name, errp);
- }
+ v->type_int(v, obj, name, errp);
}
void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
{
- int64_t value;
-
if (v->type_size) {
v->type_size(v, obj, name, errp);
- } else if (v->type_uint64) {
- v->type_uint64(v, obj, name, errp);
} else {
- value = *obj;
- v->type_int(v, &value, name, errp);
- *obj = value;
+ visit_type_uint64(v, obj, name, errp);
}
}
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 11/15] qapi: Plug leaks in test-qmp-*
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (9 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 10/15] qapi: Remove dead visitor code Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 12/15] qapi: Simplify error testing " Eric Blake
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Make valgrind happy with the current state of the tests, so that
it is easier to see if future patches introduce new memory problems
without being drowned in noise. Many of the leaks were due to
calling a second init without tearing down the data from an earlier
visit. But since teardown is already idempotent, and we already
register teardown as part of input_visitor_test_add(), it is nicer
to just make init() safe to call multiple times than it is to have
to make all tests call teardown.
Another common leak was forgetting to clean up an error object,
after testing that an error was raised.
Another leak was in test_visitor_in_struct_nested(), failing to
clean the base member of UserDefTwo. Cleaning that up left
check_and_free_str() as dead code (since using the qapi_free_*
takes care of recursion, and we don't want double frees).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: make init repeatable rather than adding teardown everywhere,
fix additional leak with UserDefTwo base, plug additional files
---
tests/test-qmp-input-strict.c | 10 ++++++++++
tests/test-qmp-input-visitor.c | 41 +++++++----------------------------------
tests/test-qmp-output-visitor.c | 4 +++-
3 files changed, 20 insertions(+), 35 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index b44184f..910e2f9 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -77,6 +77,8 @@ static Visitor *validate_test_init_raw(TestInputVisitorData *data,
{
Visitor *v;
+ validate_teardown(data, NULL);
+
data->obj = qobject_from_json(json_string);
g_assert(data->obj != NULL);
@@ -193,6 +195,8 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
visit_type_TestStruct(v, &p, NULL, &err);
g_assert(err);
+ error_free(err);
+ /* FIXME: visitor should not allocate p when returning error */
if (p) {
g_free(p->string);
}
@@ -210,6 +214,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
visit_type_UserDefTwo(v, &udp, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefTwo(udp);
}
@@ -224,6 +229,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
visit_type_UserDefOneList(v, &head, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefOneList(head);
}
@@ -239,6 +245,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefNativeListUnion(tmp);
}
@@ -253,6 +260,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefFlatUnion(tmp);
}
@@ -268,6 +276,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefFlatUnion2(tmp);
}
@@ -282,6 +291,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
visit_type_UserDefAlternate(v, &tmp, NULL, &err);
g_assert(err);
+ error_free(err);
qapi_free_UserDefAlternate(tmp);
}
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 51f1fb7..70e2766 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -46,6 +46,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
Visitor *v;
va_list ap;
+ visitor_input_teardown(data, NULL);
+
va_start(ap, json_string);
data->obj = qobject_from_jsonv(json_string, &ap);
va_end(ap);
@@ -177,12 +179,7 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
visit_type_EnumOne(v, &res, NULL, &err);
g_assert(!err);
g_assert_cmpint(i, ==, res);
-
- visitor_input_teardown(data, NULL);
}
-
- data->obj = NULL;
- data->qiv = NULL;
}
@@ -205,12 +202,6 @@ static void test_visitor_in_struct(TestInputVisitorData *data,
g_free(p);
}
-static void check_and_free_str(char *str, const char *cmp)
-{
- g_assert_cmpstr(str, ==, cmp);
- g_free(str);
-}
-
static void test_visitor_in_struct_nested(TestInputVisitorData *data,
const void *unused)
{
@@ -226,17 +217,14 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data,
visit_type_UserDefTwo(v, &udp, NULL, &err);
g_assert(!err);
- check_and_free_str(udp->string0, "string0");
- check_and_free_str(udp->dict1->string1, "string1");
+ g_assert_cmpstr(udp->string0, ==, "string0");
+ g_assert_cmpstr(udp->dict1->string1, ==, "string1");
g_assert_cmpint(udp->dict1->dict2->userdef->base->integer, ==, 42);
- check_and_free_str(udp->dict1->dict2->userdef->string, "string");
- check_and_free_str(udp->dict1->dict2->string, "string2");
+ g_assert_cmpstr(udp->dict1->dict2->userdef->string, ==, "string");
+ g_assert_cmpstr(udp->dict1->dict2->string, ==, "string2");
g_assert(udp->dict1->has_dict3 == false);
- g_free(udp->dict1->dict2->userdef);
- g_free(udp->dict1->dict2);
- g_free(udp->dict1);
- g_free(udp);
+ qapi_free_UserDefTwo(udp);
}
static void test_visitor_in_list(TestInputVisitorData *data,
@@ -341,14 +329,12 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
g_assert_cmpint(tmp->type, ==, QTYPE_QINT);
g_assert_cmpint(tmp->i, ==, 42);
qapi_free_UserDefAlternate(tmp);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "'string'");
visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
g_assert_cmpint(tmp->type, ==, QTYPE_QSTRING);
g_assert_cmpstr(tmp->s, ==, "string");
qapi_free_UserDefAlternate(tmp);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "false");
visit_type_UserDefAlternate(v, &tmp, NULL, &err);
@@ -356,7 +342,6 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
error_free(err);
err = NULL;
qapi_free_UserDefAlternate(tmp);
- visitor_input_teardown(data, NULL);
}
static void test_visitor_in_alternate_number(TestInputVisitorData *data,
@@ -379,42 +364,36 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
error_free(err);
err = NULL;
qapi_free_AltStrBool(asb);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltStrNum(v, &asn, NULL, &error_abort);
g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(asn->n, ==, 42);
qapi_free_AltStrNum(asn);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltNumStr(v, &ans, NULL, &error_abort);
g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ans->n, ==, 42);
qapi_free_AltNumStr(ans);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltStrInt(v, &asi, NULL, &error_abort);
g_assert_cmpint(asi->type, ==, QTYPE_QINT);
g_assert_cmpint(asi->i, ==, 42);
qapi_free_AltStrInt(asi);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
g_assert_cmpint(ain->type, ==, QTYPE_QINT);
g_assert_cmpint(ain->i, ==, 42);
qapi_free_AltIntNum(ain);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
g_assert_cmpint(ani->type, ==, QTYPE_QINT);
g_assert_cmpint(ani->i, ==, 42);
qapi_free_AltNumInt(ani);
- visitor_input_teardown(data, NULL);
/* Parsing a double */
@@ -424,21 +403,18 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
error_free(err);
err = NULL;
qapi_free_AltStrBool(asb);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrNum(v, &asn, NULL, &error_abort);
g_assert_cmpint(asn->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(asn->n, ==, 42.5);
qapi_free_AltStrNum(asn);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumStr(v, &ans, NULL, &error_abort);
g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ans->n, ==, 42.5);
qapi_free_AltNumStr(ans);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrInt(v, &asi, NULL, &err);
@@ -446,21 +422,18 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
error_free(err);
err = NULL;
qapi_free_AltStrInt(asi);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltIntNum(v, &ain, NULL, &error_abort);
g_assert_cmpint(ain->type, ==, QTYPE_QFLOAT);
g_assert_cmpfloat(ain->n, ==, 42.5);
qapi_free_AltIntNum(ain);
- visitor_input_teardown(data, NULL);
v = visitor_input_test_init(data, "42.5");
visit_type_AltNumInt(v, &ani, NULL, &error_abort);
g_assert_cmpint(ani->type, ==, QTYPE_QFLOAT);
g_assert_cmpint(ani->n, ==, 42.5);
qapi_free_AltNumInt(ani);
- visitor_input_teardown(data, NULL);
}
static void test_native_list_integer_helper(TestInputVisitorData *data,
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index e72b22c..dd5ce14 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -398,6 +398,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
qobj = QOBJECT(qdict);
visit_type_any(data->ov, &qobj, NULL, &err);
g_assert(!err);
+ qobject_decref(qobj);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
qdict = qobject_to_qdict(obj);
@@ -418,7 +419,6 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
g_assert(qstring);
g_assert_cmpstr(qstring_get_str(qstring), ==, "foo");
qobject_decref(obj);
- qobject_decref(qobj);
}
static void test_visitor_out_union_flat(TestOutputVisitorData *data,
@@ -469,6 +469,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);
qapi_free_UserDefAlternate(tmp);
+ qobject_decref(arg);
tmp = g_malloc0(sizeof(UserDefAlternate));
tmp->type = QTYPE_QSTRING;
@@ -481,6 +482,7 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
g_assert_cmpstr(qstring_get_str(qobject_to_qstring(arg)), ==, "hello");
qapi_free_UserDefAlternate(tmp);
+ qobject_decref(arg);
}
static void test_visitor_out_empty(TestOutputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 12/15] qapi: Simplify error testing in test-qmp-*
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (10 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 11/15] qapi: Plug leaks in test-qmp-* Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 13/15] qapi: Test failure in middle of array parse Eric Blake
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
By using &error_abort, we can avoid a local err variable in
situations where we expect success.
By moving err into data, we can let test teardown take care
of cleaning up any collected error (and allowing for fewer
lines of code between repeated tests where init runs teardown
on our behalf).
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
tests/test-qmp-input-strict.c | 77 +++++++++-------------------
tests/test-qmp-input-visitor.c | 101 ++++++++++++-------------------------
tests/test-qmp-output-visitor.c | 52 +++++--------------
tests/test-visitor-serialization.c | 42 +++++++--------
4 files changed, 86 insertions(+), 186 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 910e2f9..f8da75c 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -26,6 +26,7 @@
typedef struct TestInputVisitorData {
QObject *obj;
QmpInputVisitor *qiv;
+ Error *err;
} TestInputVisitorData;
static void validate_teardown(TestInputVisitorData *data,
@@ -34,6 +35,9 @@ static void validate_teardown(TestInputVisitorData *data,
qobject_decref(data->obj);
data->obj = NULL;
+ error_free(data->err);
+ data->err = NULL;
+
if (data->qiv) {
qmp_input_visitor_cleanup(data->qiv);
data->qiv = NULL;
@@ -96,13 +100,11 @@ static void test_validate_struct(TestInputVisitorData *data,
const void *unused)
{
TestStruct *p = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");
- visit_type_TestStruct(v, &p, NULL, &err);
- g_assert(!err);
+ visit_type_TestStruct(v, &p, NULL, &error_abort);
g_free(p->string);
g_free(p);
}
@@ -111,7 +113,6 @@ static void test_validate_struct_nested(TestInputVisitorData *data,
const void *unused)
{
UserDefTwo *udp = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "{ 'string0': 'string0', "
@@ -119,8 +120,7 @@ static void test_validate_struct_nested(TestInputVisitorData *data,
"'dict2': { 'userdef': { 'integer': 42, "
"'string': 'string' }, 'string': 'string2'}}}");
- visit_type_UserDefTwo(v, &udp, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefTwo(v, &udp, NULL, &error_abort);
qapi_free_UserDefTwo(udp);
}
@@ -128,13 +128,11 @@ static void test_validate_list(TestInputVisitorData *data,
const void *unused)
{
UserDefOneList *head = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]");
- visit_type_UserDefOneList(v, &head, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefOneList(v, &head, NULL, &error_abort);
qapi_free_UserDefOneList(head);
}
@@ -143,12 +141,10 @@ static void test_validate_union_native_list(TestInputVisitorData *data,
{
UserDefNativeListUnion *tmp = NULL;
Visitor *v;
- Error *err = NULL;
v = validate_test_init(data, "{ 'type': 'integer', 'data' : [ 1, 2 ] }");
- visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefNativeListUnion(v, &tmp, NULL, &error_abort);
qapi_free_UserDefNativeListUnion(tmp);
}
@@ -157,7 +153,6 @@ static void test_validate_union_flat(TestInputVisitorData *data,
{
UserDefFlatUnion *tmp = NULL;
Visitor *v;
- Error *err = NULL;
v = validate_test_init(data,
"{ 'enum1': 'value1', "
@@ -165,8 +160,7 @@ static void test_validate_union_flat(TestInputVisitorData *data,
"'string': 'str', "
"'boolean': true }");
- visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefFlatUnion(v, &tmp, NULL, &error_abort);
qapi_free_UserDefFlatUnion(tmp);
}
@@ -175,12 +169,10 @@ static void test_validate_alternate(TestInputVisitorData *data,
{
UserDefAlternate *tmp = NULL;
Visitor *v;
- Error *err = NULL;
v = validate_test_init(data, "42");
- visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
qapi_free_UserDefAlternate(tmp);
}
@@ -188,14 +180,12 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
const void *unused)
{
TestStruct *p = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo', 'extra': 42 }");
- visit_type_TestStruct(v, &p, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_TestStruct(v, &p, NULL, &data->err);
+ g_assert(data->err);
/* FIXME: visitor should not allocate p when returning error */
if (p) {
g_free(p->string);
@@ -207,14 +197,12 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
const void *unused)
{
UserDefTwo *udp = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "{ 'string0': 'string0', 'dict1': { 'string1': 'string1', 'dict2': { 'userdef1': { 'integer': 42, 'string': 'string', 'extra': [42, 23, {'foo':'bar'}] }, 'string2': 'string2'}}}");
- visit_type_UserDefTwo(v, &udp, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefTwo(v, &udp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefTwo(udp);
}
@@ -222,14 +210,12 @@ static void test_validate_fail_list(TestInputVisitorData *data,
const void *unused)
{
UserDefOneList *head = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44, 'extra': 'ggg' } ]");
- visit_type_UserDefOneList(v, &head, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefOneList(v, &head, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefOneList(head);
}
@@ -237,15 +223,13 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
const void *unused)
{
UserDefNativeListUnion *tmp = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data,
"{ 'type': 'integer', 'data' : [ 'string' ] }");
- visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefNativeListUnion(v, &tmp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefNativeListUnion(tmp);
}
@@ -253,14 +237,12 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
const void *unused)
{
UserDefFlatUnion *tmp = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
- visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefFlatUnion(v, &tmp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefFlatUnion(tmp);
}
@@ -268,15 +250,13 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
const void *unused)
{
UserDefFlatUnion2 *tmp = NULL;
- Error *err = NULL;
Visitor *v;
/* test situation where discriminator field ('enum1' here) is missing */
v = validate_test_init(data, "{ 'integer': 42, 'string': 'c', 'string1': 'd', 'string2': 'e' }");
- visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefFlatUnion2(v, &tmp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefFlatUnion2(tmp);
}
@@ -285,13 +265,11 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
{
UserDefAlternate *tmp = NULL;
Visitor *v;
- Error *err = NULL;
v = validate_test_init(data, "3.14");
- visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_UserDefAlternate(v, &tmp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefAlternate(tmp);
}
@@ -299,16 +277,11 @@ static void do_test_validate_qmp_introspect(TestInputVisitorData *data,
const char *schema_json)
{
SchemaInfoList *schema = NULL;
- Error *err = NULL;
Visitor *v;
v = validate_test_init_raw(data, schema_json);
- visit_type_SchemaInfoList(v, &schema, NULL, &err);
- if (err) {
- fprintf(stderr, "%s", error_get_pretty(err));
- }
- g_assert(!err);
+ visit_type_SchemaInfoList(v, &schema, NULL, &error_abort);
g_assert(schema);
qapi_free_SchemaInfoList(schema);
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 70e2766..6b1728c 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -22,6 +22,7 @@
typedef struct TestInputVisitorData {
QObject *obj;
QmpInputVisitor *qiv;
+ Error *err;
} TestInputVisitorData;
static void visitor_input_teardown(TestInputVisitorData *data,
@@ -30,6 +31,9 @@ static void visitor_input_teardown(TestInputVisitorData *data,
qobject_decref(data->obj);
data->obj = NULL;
+ error_free(data->err);
+ data->err = NULL;
+
if (data->qiv) {
qmp_input_visitor_cleanup(data->qiv);
data->qiv = NULL;
@@ -92,13 +96,11 @@ static void test_visitor_in_int(TestInputVisitorData *data,
const void *unused)
{
int64_t res = 0, value = -42;
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "%" PRId64, value);
- visit_type_int(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_int(v, &res, NULL, &error_abort);
g_assert_cmpint(res, ==, value);
}
@@ -106,7 +108,6 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
const void *unused)
{
int64_t res = 0;
- Error *err = NULL;
Visitor *v;
/* this will overflow a Qint/int64, so should be deserialized into
@@ -115,22 +116,19 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
*/
v = visitor_input_test_init(data, "%f", DBL_MAX);
- visit_type_int(v, &res, NULL, &err);
- g_assert(err);
- error_free(err);
+ visit_type_int(v, &res, NULL, &data->err);
+ g_assert(data->err);
}
static void test_visitor_in_bool(TestInputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
bool res = false;
Visitor *v;
v = visitor_input_test_init(data, "true");
- visit_type_bool(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_bool(v, &res, NULL, &error_abort);
g_assert_cmpint(res, ==, true);
}
@@ -138,13 +136,11 @@ static void test_visitor_in_number(TestInputVisitorData *data,
const void *unused)
{
double res = 0, value = 3.14;
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "%f", value);
- visit_type_number(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_number(v, &res, NULL, &error_abort);
g_assert_cmpfloat(res, ==, value);
}
@@ -152,13 +148,11 @@ static void test_visitor_in_string(TestInputVisitorData *data,
const void *unused)
{
char *res = NULL, *value = (char *) "Q E M U";
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "%s", value);
- visit_type_str(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_str(v, &res, NULL, &error_abort);
g_assert_cmpstr(res, ==, value);
g_free(res);
@@ -167,7 +161,6 @@ static void test_visitor_in_string(TestInputVisitorData *data,
static void test_visitor_in_enum(TestInputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
Visitor *v;
EnumOne i;
@@ -176,8 +169,7 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
v = visitor_input_test_init(data, "%s", EnumOne_lookup[i]);
- visit_type_EnumOne(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_EnumOne(v, &res, NULL, &error_abort);
g_assert_cmpint(i, ==, res);
}
}
@@ -187,13 +179,11 @@ static void test_visitor_in_struct(TestInputVisitorData *data,
const void *unused)
{
TestStruct *p = NULL;
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");
- visit_type_TestStruct(v, &p, NULL, &err);
- g_assert(!err);
+ visit_type_TestStruct(v, &p, NULL, &error_abort);
g_assert_cmpint(p->integer, ==, -42);
g_assert(p->boolean == true);
g_assert_cmpstr(p->string, ==, "foo");
@@ -206,7 +196,6 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data,
const void *unused)
{
UserDefTwo *udp = NULL;
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "{ 'string0': 'string0', "
@@ -214,8 +203,7 @@ static void test_visitor_in_struct_nested(TestInputVisitorData *data,
"'dict2': { 'userdef': { 'integer': 42, "
"'string': 'string' }, 'string': 'string2'}}}");
- visit_type_UserDefTwo(v, &udp, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefTwo(v, &udp, NULL, &error_abort);
g_assert_cmpstr(udp->string0, ==, "string0");
g_assert_cmpstr(udp->dict1->string1, ==, "string1");
@@ -231,14 +219,12 @@ static void test_visitor_in_list(TestInputVisitorData *data,
const void *unused)
{
UserDefOneList *item, *head = NULL;
- Error *err = NULL;
Visitor *v;
int i;
v = visitor_input_test_init(data, "[ { 'string': 'string0', 'integer': 42 }, { 'string': 'string1', 'integer': 43 }, { 'string': 'string2', 'integer': 44 } ]");
- visit_type_UserDefOneList(v, &head, NULL, &err);
- g_assert(!err);
+ visit_type_UserDefOneList(v, &head, NULL, &error_abort);
g_assert(head != NULL);
for (i = 0, item = head; item; item = item->next, i++) {
@@ -256,7 +242,6 @@ static void test_visitor_in_any(TestInputVisitorData *data,
const void *unused)
{
QObject *res = NULL;
- Error *err = NULL;
Visitor *v;
QInt *qint;
QBool *qbool;
@@ -265,16 +250,14 @@ static void test_visitor_in_any(TestInputVisitorData *data,
QObject *qobj;
v = visitor_input_test_init(data, "-42");
- visit_type_any(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_any(v, &res, NULL, &error_abort);
qint = qobject_to_qint(res);
g_assert(qint);
g_assert_cmpint(qint_get_int(qint), ==, -42);
qobject_decref(res);
v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true, 'string': 'foo' }");
- visit_type_any(v, &res, NULL, &err);
- g_assert(!err);
+ visit_type_any(v, &res, NULL, &error_abort);
qdict = qobject_to_qdict(res);
g_assert(qdict && qdict_size(qdict) == 3);
qobj = qdict_get(qdict, "integer");
@@ -299,7 +282,6 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
const void *unused)
{
Visitor *v;
- Error *err = NULL;
UserDefFlatUnion *tmp;
v = visitor_input_test_init(data,
@@ -308,8 +290,7 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
"'string': 'str', "
"'boolean': true }");
- visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefFlatUnion(v, &tmp, NULL, &error_abort);
g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
g_assert_cmpstr(tmp->string, ==, "str");
g_assert_cmpint(tmp->integer, ==, 41);
@@ -321,7 +302,6 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
const void *unused)
{
Visitor *v;
- Error *err = NULL;
UserDefAlternate *tmp;
v = visitor_input_test_init(data, "42");
@@ -337,10 +317,8 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
qapi_free_UserDefAlternate(tmp);
v = visitor_input_test_init(data, "false");
- visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_UserDefAlternate(v, &tmp, NULL, &data->err);
+ g_assert(data->err);
qapi_free_UserDefAlternate(tmp);
}
@@ -348,7 +326,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
const void *unused)
{
Visitor *v;
- Error *err = NULL;
AltStrBool *asb;
AltStrNum *asn;
AltNumStr *ans;
@@ -359,10 +336,8 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
/* Parsing an int */
v = visitor_input_test_init(data, "42");
- visit_type_AltStrBool(v, &asb, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_AltStrBool(v, &asb, NULL, &data->err);
+ g_assert(data->err);
qapi_free_AltStrBool(asb);
v = visitor_input_test_init(data, "42");
@@ -398,10 +373,8 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
/* Parsing a double */
v = visitor_input_test_init(data, "42.5");
- visit_type_AltStrBool(v, &asb, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_AltStrBool(v, &asb, NULL, &data->err);
+ g_assert(data->err);
qapi_free_AltStrBool(asb);
v = visitor_input_test_init(data, "42.5");
@@ -417,10 +390,8 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
qapi_free_AltNumStr(ans);
v = visitor_input_test_init(data, "42.5");
- visit_type_AltStrInt(v, &asi, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ visit_type_AltStrInt(v, &asi, NULL, &data->err);
+ g_assert(data->err);
qapi_free_AltStrInt(asi);
v = visitor_input_test_init(data, "42.5");
@@ -441,7 +412,6 @@ static void test_native_list_integer_helper(TestInputVisitorData *data,
UserDefNativeListUnionKind kind)
{
UserDefNativeListUnion *cvalue = NULL;
- Error *err = NULL;
Visitor *v;
GString *gstr_list = g_string_new("");
GString *gstr_union = g_string_new("");
@@ -458,8 +428,7 @@ static void test_native_list_integer_helper(TestInputVisitorData *data,
gstr_list->str);
v = visitor_input_test_init_raw(data, gstr_union->str);
- visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->kind, ==, kind);
@@ -604,7 +573,6 @@ static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
{
UserDefNativeListUnion *cvalue = NULL;
boolList *elem = NULL;
- Error *err = NULL;
Visitor *v;
GString *gstr_list = g_string_new("");
GString *gstr_union = g_string_new("");
@@ -621,8 +589,7 @@ static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
gstr_list->str);
v = visitor_input_test_init_raw(data, gstr_union->str);
- visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
@@ -640,7 +607,6 @@ static void test_visitor_in_native_list_string(TestInputVisitorData *data,
{
UserDefNativeListUnion *cvalue = NULL;
strList *elem = NULL;
- Error *err = NULL;
Visitor *v;
GString *gstr_list = g_string_new("");
GString *gstr_union = g_string_new("");
@@ -656,8 +622,7 @@ static void test_visitor_in_native_list_string(TestInputVisitorData *data,
gstr_list->str);
v = visitor_input_test_init_raw(data, gstr_union->str);
- visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
@@ -679,7 +644,6 @@ static void test_visitor_in_native_list_number(TestInputVisitorData *data,
{
UserDefNativeListUnion *cvalue = NULL;
numberList *elem = NULL;
- Error *err = NULL;
Visitor *v;
GString *gstr_list = g_string_new("");
GString *gstr_union = g_string_new("");
@@ -695,8 +659,7 @@ static void test_visitor_in_native_list_number(TestInputVisitorData *data,
gstr_list->str);
v = visitor_input_test_init_raw(data, gstr_union->str);
- visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &error_abort);
g_assert(cvalue != NULL);
g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
@@ -729,18 +692,16 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
const void *unused)
{
TestStruct *p = NULL;
- Error *err = NULL;
Visitor *v;
v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
- visit_type_TestStruct(v, &p, NULL, &err);
- g_assert(err);
+ visit_type_TestStruct(v, &p, NULL, &data->err);
+ g_assert(data->err);
/* FIXME - a failed parse should not leave a partially-allocated p
* for us to clean up; this could cause callers to leak memory. */
g_assert(p->string == NULL);
- error_free(err);
g_free(p->string);
g_free(p);
}
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index dd5ce14..f9a5db0 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -45,11 +45,9 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
const void *unused)
{
int64_t value = -42;
- Error *err = NULL;
QObject *obj;
- visit_type_int(data->ov, &value, NULL, &err);
- g_assert(!err);
+ visit_type_int(data->ov, &value, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -62,12 +60,10 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
static void test_visitor_out_bool(TestOutputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
bool value = true;
QObject *obj;
- visit_type_bool(data->ov, &value, NULL, &err);
- g_assert(!err);
+ visit_type_bool(data->ov, &value, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -81,11 +77,9 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
const void *unused)
{
double value = 3.14;
- Error *err = NULL;
QObject *obj;
- visit_type_number(data->ov, &value, NULL, &err);
- g_assert(!err);
+ visit_type_number(data->ov, &value, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -99,11 +93,9 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
const void *unused)
{
char *string = (char *) "Q E M U";
- Error *err = NULL;
QObject *obj;
- visit_type_str(data->ov, &string, NULL, &err);
- g_assert(!err);
+ visit_type_str(data->ov, &string, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -117,12 +109,10 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
const void *unused)
{
char *string = NULL;
- Error *err = NULL;
QObject *obj;
/* A null string should return "" */
- visit_type_str(data->ov, &string, NULL, &err);
- g_assert(!err);
+ visit_type_str(data->ov, &string, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -135,13 +125,11 @@ static void test_visitor_out_no_string(TestOutputVisitorData *data,
static void test_visitor_out_enum(TestOutputVisitorData *data,
const void *unused)
{
- Error *err = NULL;
QObject *obj;
EnumOne i;
for (i = 0; i < ENUM_ONE_MAX; i++) {
- visit_type_EnumOne(data->ov, &i, "unused", &err);
- g_assert(!err);
+ visit_type_EnumOne(data->ov, &i, "unused", &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -174,12 +162,10 @@ static void test_visitor_out_struct(TestOutputVisitorData *data,
.boolean = false,
.string = (char *) "foo"};
TestStruct *p = &test_struct;
- Error *err = NULL;
QObject *obj;
QDict *qdict;
- visit_type_TestStruct(data->ov, &p, NULL, &err);
- g_assert(!err);
+ visit_type_TestStruct(data->ov, &p, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -198,7 +184,6 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
const void *unused)
{
int64_t value = 42;
- Error *err = NULL;
UserDefTwo *ud2;
QObject *obj;
QDict *qdict, *dict1, *dict2, *dict3, *userdef;
@@ -227,8 +212,7 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
ud2->dict1->dict3->userdef->base->integer = value;
ud2->dict1->dict3->string = g_strdup(strings[3]);
- visit_type_UserDefTwo(data->ov, &ud2, "unused", &err);
- g_assert(!err);
+ visit_type_UserDefTwo(data->ov, &ud2, "unused", &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -290,7 +274,6 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
const int max_items = 10;
bool value_bool = true;
int value_int = 10;
- Error *err = NULL;
QListEntry *entry;
QObject *obj;
QList *qlist;
@@ -307,8 +290,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
head = p;
}
- visit_type_TestStructList(data->ov, &head, NULL, &err);
- g_assert(!err);
+ visit_type_TestStructList(data->ov, &head, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -374,7 +356,6 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
const void *unused)
{
QObject *qobj;
- Error *err = NULL;
QInt *qint;
QBool *qbool;
QString *qstring;
@@ -382,8 +363,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
QObject *obj;
qobj = QOBJECT(qint_from_int(-42));
- visit_type_any(data->ov, &qobj, NULL, &err);
- g_assert(!err);
+ visit_type_any(data->ov, &qobj, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
g_assert(qobject_type(obj) == QTYPE_QINT);
@@ -396,8 +376,7 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
qdict_put(qdict, "boolean", qbool_from_bool(true));
qdict_put(qdict, "string", qstring_from_str("foo"));
qobj = QOBJECT(qdict);
- visit_type_any(data->ov, &qobj, NULL, &err);
- g_assert(!err);
+ visit_type_any(data->ov, &qobj, NULL, &error_abort);
qobject_decref(qobj);
obj = qmp_output_get_qobject(data->qov);
g_assert(obj != NULL);
@@ -427,8 +406,6 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
QObject *arg;
QDict *qdict;
- Error *err = NULL;
-
UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
tmp->enum1 = ENUM_ONE_VALUE1;
tmp->string = g_strdup("str");
@@ -436,8 +413,7 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
/* TODO when generator bug is fixed: tmp->integer = 41; */
tmp->value1->boolean = true;
- visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &error_abort);
arg = qmp_output_get_qobject(data->qov);
g_assert(qobject_type(arg) == QTYPE_QDICT);
@@ -716,14 +692,12 @@ static void test_native_list(TestOutputVisitorData *data,
UserDefNativeListUnionKind kind)
{
UserDefNativeListUnion *cvalue = g_new0(UserDefNativeListUnion, 1);
- Error *err = NULL;
QObject *obj;
cvalue->kind = kind;
init_native_list(cvalue);
- visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &err);
- g_assert(err == NULL);
+ visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &error_abort);
obj = qmp_output_get_qobject(data->qov);
check_native_list(obj, cvalue->kind);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 77ab21f..6f28269 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -304,14 +304,13 @@ static void test_primitives(gconstpointer opaque)
const SerializeOps *ops = args->ops;
PrimitiveType *pt = args->test_data;
PrimitiveType *pt_copy = g_malloc0(sizeof(*pt_copy));
- Error *err = NULL;
void *serialize_data;
pt_copy->type = pt->type;
- ops->serialize(pt, &serialize_data, visit_primitive_type, &err);
- ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type, &err);
+ ops->serialize(pt, &serialize_data, visit_primitive_type, &error_abort);
+ ops->deserialize((void **)&pt_copy, serialize_data, visit_primitive_type,
+ &error_abort);
- g_assert(err == NULL);
g_assert(pt_copy != NULL);
if (pt->type == PTYPE_STRING) {
g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
@@ -347,7 +346,6 @@ static void test_primitive_lists(gconstpointer opaque)
PrimitiveList pl = { .value = { NULL } };
PrimitiveList pl_copy = { .value = { NULL } };
PrimitiveList *pl_copy_ptr = &pl_copy;
- Error *err = NULL;
void *serialize_data;
void *cur_head = NULL;
int i;
@@ -494,10 +492,11 @@ static void test_primitive_lists(gconstpointer opaque)
}
}
- ops->serialize((void **)&pl, &serialize_data, visit_primitive_list, &err);
- ops->deserialize((void **)&pl_copy_ptr, serialize_data, visit_primitive_list, &err);
+ ops->serialize((void **)&pl, &serialize_data, visit_primitive_list,
+ &error_abort);
+ ops->deserialize((void **)&pl_copy_ptr, serialize_data,
+ visit_primitive_list, &error_abort);
- g_assert(err == NULL);
i = 0;
/* compare our deserialized list of primitives to the original */
@@ -654,10 +653,8 @@ static void test_primitive_lists(gconstpointer opaque)
g_assert_cmpint(i, ==, 33);
ops->cleanup(serialize_data);
- dealloc_helper(&pl, visit_primitive_list, &err);
- g_assert(!err);
- dealloc_helper(&pl_copy, visit_primitive_list, &err);
- g_assert(!err);
+ dealloc_helper(&pl, visit_primitive_list, &error_abort);
+ dealloc_helper(&pl_copy, visit_primitive_list, &error_abort);
g_free(args);
}
@@ -667,13 +664,12 @@ static void test_struct(gconstpointer opaque)
const SerializeOps *ops = args->ops;
TestStruct *ts = struct_create();
TestStruct *ts_copy = NULL;
- Error *err = NULL;
void *serialize_data;
- ops->serialize(ts, &serialize_data, visit_struct, &err);
- ops->deserialize((void **)&ts_copy, serialize_data, visit_struct, &err);
+ ops->serialize(ts, &serialize_data, visit_struct, &error_abort);
+ ops->deserialize((void **)&ts_copy, serialize_data, visit_struct,
+ &error_abort);
- g_assert(err == NULL);
struct_compare(ts, ts_copy);
struct_cleanup(ts);
@@ -689,14 +685,12 @@ static void test_nested_struct(gconstpointer opaque)
const SerializeOps *ops = args->ops;
UserDefTwo *udnp = nested_struct_create();
UserDefTwo *udnp_copy = NULL;
- Error *err = NULL;
void *serialize_data;
- ops->serialize(udnp, &serialize_data, visit_nested_struct, &err);
+ ops->serialize(udnp, &serialize_data, visit_nested_struct, &error_abort);
ops->deserialize((void **)&udnp_copy, serialize_data, visit_nested_struct,
- &err);
+ &error_abort);
- g_assert(err == NULL);
nested_struct_compare(udnp, udnp_copy);
nested_struct_cleanup(udnp);
@@ -711,7 +705,6 @@ static void test_nested_struct_list(gconstpointer opaque)
TestArgs *args = (TestArgs *) opaque;
const SerializeOps *ops = args->ops;
UserDefTwoList *listp = NULL, *tmp, *tmp_copy, *listp_copy = NULL;
- Error *err = NULL;
void *serialize_data;
int i = 0;
@@ -722,11 +715,10 @@ static void test_nested_struct_list(gconstpointer opaque)
listp = tmp;
}
- ops->serialize(listp, &serialize_data, visit_nested_struct_list, &err);
+ ops->serialize(listp, &serialize_data, visit_nested_struct_list,
+ &error_abort);
ops->deserialize((void **)&listp_copy, serialize_data,
- visit_nested_struct_list, &err);
-
- g_assert(err == NULL);
+ visit_nested_struct_list, &error_abort);
tmp = listp;
tmp_copy = listp_copy;
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 13/15] qapi: Test failure in middle of array parse
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (11 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 12/15] qapi: Simplify error testing " Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 14/15] qapi: More tests of input arrays Eric Blake
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
Our generated list visitors have the same problem as has been
mentioned elsewhere (see commit 2f52e20): they allocate data
even on failure. An upcoming patch will correct things to
provide saner guarantees, but first we need to expose the
behavior in the testsuite to ensure we aren't introducing any
memory usage bugs.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase onto earlier gen_err_check() and testsuite improvements
---
scripts/qapi-visit.py | 4 ++++
tests/test-qmp-input-visitor.c | 10 ++++++++++
2 files changed, 14 insertions(+)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 1ac5350..d7f7f8b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -128,6 +128,10 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
def gen_visit_list(name, element_type):
+ # FIXME: if *obj is NULL on entry, and the first visit_next_list()
+ # assigns to *obj, while a later one fails, we should clean up *obj
+ # rather than leaving it non-NULL. As currently written, the caller must
+ # call qapi_free_FOOList() to avoid a memory leak of the partial FOOList.
return mcgen('''
void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error **errp)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 6b1728c..8dbc3bf 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -693,6 +693,7 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
{
TestStruct *p = NULL;
Visitor *v;
+ strList *q = NULL;
v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
@@ -704,6 +705,15 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
g_free(p->string);
g_free(p);
+
+ v = visitor_input_test_init(data, "[ '1', '2', false, '3' ]");
+ /* FIXME - a failed parse should not leave a partially-allocated
+ * array for us to clean up; this could cause callers to leak
+ * memory. */
+ visit_type_strList(v, &q, NULL, &data->err);
+ assert(q);
+ assert(data->err);
+ qapi_free_strList(q);
}
int main(int argc, char **argv)
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 14/15] qapi: More tests of input arrays
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (12 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 13/15] qapi: Test failure in middle of array parse Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 15/15] qapi: Simplify visits of optional fields Eric Blake
2015-10-16 15:23 ` [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
Our testsuite had no coverage of empty arrays, nor of what
happens when the input does not match the expected type.
Useful to have, especially if we start changing the visitor
contracts.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: new patch
---
tests/test-qmp-input-visitor.c | 51 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 8dbc3bf..6972ee8 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -236,6 +236,12 @@ static void test_visitor_in_list(TestInputVisitorData *data,
}
qapi_free_UserDefOneList(head);
+ head = NULL;
+
+ /* An empty list is valid */
+ v = visitor_input_test_init(data, "[]");
+ visit_type_UserDefOneList(v, &head, NULL, &error_abort);
+ g_assert(!head);
}
static void test_visitor_in_any(TestInputVisitorData *data,
@@ -716,6 +722,49 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
qapi_free_strList(q);
}
+static void test_visitor_in_wrong_type(TestInputVisitorData *data,
+ const void *unused)
+{
+ TestStruct *p = NULL;
+ Visitor *v;
+ strList *q = NULL;
+ int64_t i;
+
+ /* Make sure arrays and structs cannot be confused */
+
+ v = visitor_input_test_init(data, "[]");
+ visit_type_TestStruct(v, &p, NULL, &data->err);
+ g_assert(data->err);
+ g_assert(!p);
+
+ v = visitor_input_test_init(data, "{}");
+ visit_type_strList(v, &q, NULL, &data->err);
+ assert(data->err);
+ assert(!q);
+
+ /* Make sure primitives and struct cannot be confused */
+
+ v = visitor_input_test_init(data, "1");
+ visit_type_TestStruct(v, &p, NULL, &data->err);
+ g_assert(data->err);
+ g_assert(!p);
+
+ v = visitor_input_test_init(data, "{}");
+ visit_type_int(v, &i, NULL, &data->err);
+ assert(data->err);
+
+ /* Make sure primitives and arrays cannot be confused */
+
+ v = visitor_input_test_init(data, "1");
+ visit_type_strList(v, &q, NULL, &data->err);
+ assert(data->err);
+ assert(!q);
+
+ v = visitor_input_test_init(data, "[]");
+ visit_type_int(v, &i, NULL, &data->err);
+ assert(data->err);
+}
+
int main(int argc, char **argv)
{
TestInputVisitorData in_visitor_data;
@@ -748,6 +797,8 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_alternate);
input_visitor_test_add("/visitor/input/errors",
&in_visitor_data, test_visitor_in_errors);
+ input_visitor_test_add("/visitor/input/wrong-type",
+ &in_visitor_data, test_visitor_in_wrong_type);
input_visitor_test_add("/visitor/input/alternate-number",
&in_visitor_data, test_visitor_in_alternate_number);
input_visitor_test_add("/visitor/input/native_list/int",
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 15/15] qapi: Simplify visits of optional fields
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (13 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 14/15] qapi: More tests of input arrays Eric Blake
@ 2015-10-08 2:00 ` Eric Blake
2015-10-16 15:23 ` [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-08 2:00 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru, Michael Roth
None of the visitor callbacks would set an error when testing
if an optional field was present; make this part of the interface
contract by eliminating the errp argument. Then, for less code,
reflect the determined boolean value back to the caller instead
of making the caller read the boolean after the fact.
The resulting generated code has a nice diff:
|- visit_optional(v, &has_fdset_id, "fdset-id", &err);
|- if (err) {
|- goto out;
|- }
|- if (has_fdset_id) {
|+ if (visit_optional(v, &has_fdset_id, "fdset-id")) {
| visit_type_int(v, &fdset_id, "fdset-id", &err);
| if (err) {
| goto out;
| }
| }
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v6: rebase onto earlier testsuite and gen_err_check() improvements
---
include/qapi/visitor-impl.h | 5 ++---
include/qapi/visitor.h | 10 ++++++++--
qapi/opts-visitor.c | 2 +-
qapi/qapi-visit-core.c | 6 +++---
qapi/qmp-input-visitor.c | 3 +--
qapi/string-input-visitor.c | 3 +--
scripts/qapi.py | 9 ++-------
7 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 370935a..fd2e905 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -44,9 +44,8 @@ struct Visitor
void (*type_any)(Visitor *v, QObject **obj, const char *name,
Error **errp);
- /* May be NULL */
- void (*optional)(Visitor *v, bool *present, const char *name,
- Error **errp);
+ /* May be NULL; most useful for input visitors. */
+ void (*optional)(Visitor *v, bool *present, const char *name);
bool (*start_union)(Visitor *v, bool data_present, Error **errp);
void (*end_union)(Visitor *v, bool data_present, Error **errp);
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 67ddd83..e52ad39 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -36,8 +36,14 @@ void visit_end_implicit_struct(Visitor *v, Error **errp);
void visit_start_list(Visitor *v, const char *name, Error **errp);
GenericList *visit_next_list(Visitor *v, GenericList **list, Error **errp);
void visit_end_list(Visitor *v, Error **errp);
-void visit_optional(Visitor *v, bool *present, const char *name,
- Error **errp);
+
+/**
+ * Check if an optional member @name of an object needs visiting.
+ * For input visitors, set *@present according to whether the
+ * corresponding visit_type_*() needs calling; for other visitors,
+ * leave *@present unchanged. Return *@present for convenience.
+ */
+bool visit_optional(Visitor *v, bool *present, const char *name);
/**
* Determine the qtype of the item @name in the current object visit.
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index cd10392..ef5fb8b 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -488,7 +488,7 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp)
static void
-opts_optional(Visitor *v, bool *present, const char *name, Error **errp)
+opts_optional(Visitor *v, bool *present, const char *name)
{
OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v);
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index cbf7780..2594147 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -73,12 +73,12 @@ void visit_end_union(Visitor *v, bool data_present, Error **errp)
}
}
-void visit_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+bool visit_optional(Visitor *v, bool *present, const char *name)
{
if (v->optional) {
- v->optional(v, present, name, errp);
+ v->optional(v, present, name);
}
+ return *present;
}
void visit_get_next_type(Visitor *v, qtype_code *type, bool promote_int,
diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c
index 5310db5..f714dfc 100644
--- a/qapi/qmp-input-visitor.c
+++ b/qapi/qmp-input-visitor.c
@@ -300,8 +300,7 @@ static void qmp_input_type_any(Visitor *v, QObject **obj, const char *name,
*obj = qobj;
}
-static void qmp_input_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+static void qmp_input_optional(Visitor *v, bool *present, const char *name)
{
QmpInputVisitor *qiv = to_qiv(v);
QObject *qobj = qmp_input_get_object(qiv, name, true);
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index bbd6a54..dee780a 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -299,8 +299,7 @@ static void parse_type_number(Visitor *v, double *obj, const char *name,
*obj = val;
}
-static void parse_optional(Visitor *v, bool *present, const char *name,
- Error **errp)
+static void parse_optional(Visitor *v, bool *present, const char *name)
{
StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 430c4bc..de76078 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1648,15 +1648,10 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
for memb in members:
if memb.optional:
ret += mcgen('''
- visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
+ if (visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s")) {
''',
prefix=prefix, c_name=memb.c_name(),
- name=memb.name, errp=errparg)
- ret += gen_err_check(skiperr=skiperr)
- ret += mcgen('''
- if (%(prefix)shas_%(c_name)s) {
-''',
- prefix=prefix, c_name=memb.c_name())
+ name=memb.name)
push_indent()
# Ugly: sometimes we need to cast away const
--
2.4.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C
2015-10-08 2:00 [Qemu-devel] [PATCH v6 00/15] post-introspection cleanups, subset C Eric Blake
` (14 preceding siblings ...)
2015-10-08 2:00 ` [Qemu-devel] [PATCH v6 15/15] qapi: Simplify visits of optional fields Eric Blake
@ 2015-10-16 15:23 ` Eric Blake
15 siblings, 0 replies; 17+ messages in thread
From: Eric Blake @ 2015-10-16 15:23 UTC (permalink / raw)
To: qemu-devel; +Cc: armbru
[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]
On 10/07/2015 08:00 PM, Eric Blake wrote:
> Pending prerequisite: Markus' qapi-next branch (which has my
> subset A patches):
> git://repo.or.cz/qemu/armbru.git qapi-next
> http://thread.gmane.org/gmane.comp.emulators.qemu/365827/focus=366351
> as well as my subset B patches (currently at v7):
> http://article.gmane.org/gmane.comp.emulators.qemu/366810
> http://repo.or.cz/qemu/ericb.git/shortlog/refs/tags/qapi-cleanupv7b
>
I'm about to post a rebase of this series, depending on my recent subset
B' v9, if that affects any reviewer's plans.
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg03730.html
> Also available as a tag at this location:
> git fetch git://repo.or.cz/qemu/ericb.git qapi-cleanupv6c
>
> and I plan to eventually forcefully update my branch with the rest
> of the v5 series, at:
> http://repo.or.cz/qemu/ericb.git/shortlog/refs/heads/qapi
>
> v6 notes:
> Add some patches and rebase onto work on subset B. Rearrange some
> patches from v5 (this set includes 17-20, 23, 25-27). Backport diff
> gets a bit confused by one patch title changing.
>
> 001/15:[down] 'qapi: Move empty-enum to compile-time test'
> 002/15:[down] 'qapi: Drop redundant returns-int test'
> 003/15:[down] 'qapi: Drop redundant flat-union-reverse-define test'
Meanwhile, these three already got hoisted into subset B and are in a
pending pull request.
> 004/15:[down] 'qapi: Use generated TestStruct machinery in tests'
> 005/15:[----] [--] 'qapi: Provide nicer array names in introspection'
> 006/15:[----] [--] 'qapi-introspect: Guarantee particular sorting'
> 007/15:[down] 'qapi: Change alternate layout to use 'type''
and this one may disappear completely, since v9B does the rename to type
in a cleaner fashion.
> 008/15:[0141] [FC] 'qapi: Simplify visiting of alternate types'
> 009/15:[0023] [FC] 'qapi: Fix alternates that accept 'number' but not 'int''
> 010/15:[----] [--] 'qapi: Remove dead visitor code'
> 011/15:[down] 'qapi: Plug leaks in test-qmp-*'
> 012/15:[down] 'qapi: Simplify error testing in test-qmp-*'
> 013/15:[0007] [FC] 'qapi: Test failure in middle of array parse'
> 014/15:[down] 'qapi: More tests of input arrays'
> 015/15:[0021] [FC] 'qapi: Simplify visits of optional fields'
>
> Not much direct review comments, although some of the changes here
> are updated based on comments made on other patches in the v5 series.
>
> Subset D (and more?) will come later.
>
Still true.
--
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 --]
^ permalink raw reply [flat|nested] 17+ messages in thread