* [Qemu-devel] [PULL 01/12] qapi: Use generated TestStruct machinery in tests
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 02/12] qapi: Strengthen test of TestStructList Markus Armbruster
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
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,
between the tests themselves (mainly whitespace differences, but
also a question of whether to use NULL or "TestStruct" when
calling visit_start_struct()) and from what the generator produces
(the hand-rolled versions did not cater to partially-allocated
objects, because they did not have a deallocation usage).
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>
Message-Id: <1446791754-23823-2-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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 | 34 -------------------
6 files changed, 10 insertions(+), 162 deletions(-)
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 48e104b..44638da 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
@@ -46,7 +49,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 a7e9aab..e20a823 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -92,6 +92,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
@@ -100,6 +101,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 de65982..3f6bc4d 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 09d0dd8..baf58dc 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)
@@ -314,29 +279,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 634563b..c024e5e 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)
{
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 02/12] qapi: Strengthen test of TestStructList
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 01/12] qapi: Use generated TestStruct machinery in tests Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 03/12] qobject: Protect against use-after-free in qobject_decref() Markus Armbruster
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Make each list element different, to ensure that order is
preserved, and use the generated free function instead of
hand-rolling our own to ensure (under valgrind) that the
list is properly cleaned.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-3-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-output-visitor.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index baf58dc..9364843 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -283,7 +283,7 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
static void test_visitor_out_list(TestOutputVisitorData *data,
const void *unused)
{
- char *value_str = (char *) "list value";
+ const char *value_str = "list value";
TestStructList *p, *head = NULL;
const int max_items = 10;
bool value_bool = true;
@@ -294,12 +294,13 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
QList *qlist;
int i;
+ /* Build the list in reverse order... */
for (i = 0; i < max_items; i++) {
p = g_malloc0(sizeof(*p));
p->value = g_malloc0(sizeof(*p->value));
- p->value->integer = value_int;
+ p->value->integer = value_int + (max_items - i - 1);
p->value->boolean = value_bool;
- p->value->string = value_str;
+ p->value->string = g_strdup(value_str);
p->next = head;
head = p;
@@ -315,6 +316,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
qlist = qobject_to_qlist(obj);
g_assert(!qlist_empty(qlist));
+ /* ...and ensure that the visitor sees it in order */
i = 0;
QLIST_FOREACH_ENTRY(qlist, entry) {
QDict *qdict;
@@ -322,7 +324,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
g_assert(qobject_type(entry->value) == QTYPE_QDICT);
qdict = qobject_to_qdict(entry->value);
g_assert_cmpint(qdict_size(qdict), ==, 3);
- g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, value_int);
+ g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, value_int + i);
g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, value_bool);
g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, value_str);
i++;
@@ -330,13 +332,7 @@ static void test_visitor_out_list(TestOutputVisitorData *data,
g_assert_cmpint(i, ==, max_items);
QDECREF(qlist);
-
- for (p = head; p;) {
- TestStructList *tmp = p->next;
- g_free(p->value);
- g_free(p);
- p = tmp;
- }
+ qapi_free_TestStructList(head);
}
static void test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 03/12] qobject: Protect against use-after-free in qobject_decref()
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 01/12] qapi: Use generated TestStruct machinery in tests Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 02/12] qapi: Strengthen test of TestStructList Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 04/12] qapi: Share test_init code in test-qmp-input* Markus Armbruster
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Adding an assertion to qobject_decref() will ensure that a
programming error causing use-after-free will result in
immediate failure (provided no other thread has started
using the memory) instead of silently attempting to wrap
refcnt around and leaving the problem to potentially bite
later at a harder point to diagnose.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-4-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/qmp/qobject.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index c856f55..4b96ed5 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -90,6 +90,7 @@ static inline void qobject_incref(QObject *obj)
*/
static inline void qobject_decref(QObject *obj)
{
+ assert(!obj || obj->refcnt);
if (obj && --obj->refcnt == 0) {
assert(obj->type != NULL);
assert(obj->type->destroy != NULL);
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 04/12] qapi: Share test_init code in test-qmp-input*
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (2 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 03/12] qobject: Protect against use-after-free in qobject_decref() Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 05/12] qapi: Plug leaks in test-qmp-* Markus Armbruster
` (7 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
Rather than duplicate the body of two functions just to
decide between qobject_from_jsonv() and qobject_from_json(),
exploit the fact that qobject_from_jsonv() intentionally
takes 'va_list *' instead of the more common 'va_list', and
that qobject_from_json() just calls qobject_from_jsonv(,NULL).
For each file, our two existing init functions then become
thin wrappers around a new internal function, and future
updates to initialization don't have to be duplicated.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-5-git-send-email-eblake@redhat.com>
[Two old comment typos fixed]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-input-strict.c | 48 ++++++++++++++++++++---------------------
tests/test-qmp-input-visitor.c | 49 ++++++++++++++++++++----------------------
2 files changed, 46 insertions(+), 51 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index b44184f..4837b31 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -40,9 +40,27 @@ static void validate_teardown(TestInputVisitorData *data,
}
}
-/* This is provided instead of a test setup function so that the JSON
- string used by the tests are kept in the test functions (and not
- int main()) */
+/* The various test_init functions are provided instead of a test setup
+ function so that the JSON string used by the tests are kept in the test
+ functions (and not in main()). */
+static Visitor *validate_test_init_internal(TestInputVisitorData *data,
+ const char *json_string,
+ va_list *ap)
+{
+ Visitor *v;
+
+ data->obj = qobject_from_jsonv(json_string, ap);
+ g_assert(data->obj);
+
+ data->qiv = qmp_input_visitor_new_strict(data->obj);
+ g_assert(data->qiv);
+
+ v = qmp_input_get_visitor(data->qiv);
+ g_assert(v);
+
+ return v;
+}
+
static GCC_FMT_ATTR(2, 3)
Visitor *validate_test_init(TestInputVisitorData *data,
const char *json_string, ...)
@@ -51,17 +69,8 @@ Visitor *validate_test_init(TestInputVisitorData *data,
va_list ap;
va_start(ap, json_string);
- data->obj = qobject_from_jsonv(json_string, &ap);
+ v = validate_test_init_internal(data, json_string, &ap);
va_end(ap);
-
- g_assert(data->obj != NULL);
-
- data->qiv = qmp_input_visitor_new_strict(data->obj);
- g_assert(data->qiv != NULL);
-
- v = qmp_input_get_visitor(data->qiv);
- g_assert(v != NULL);
-
return v;
}
@@ -75,18 +84,7 @@ Visitor *validate_test_init(TestInputVisitorData *data,
static Visitor *validate_test_init_raw(TestInputVisitorData *data,
const char *json_string)
{
- Visitor *v;
-
- data->obj = qobject_from_json(json_string);
- g_assert(data->obj != NULL);
-
- data->qiv = qmp_input_visitor_new_strict(data->obj);
- g_assert(data->qiv != NULL);
-
- v = qmp_input_get_visitor(data->qiv);
- g_assert(v != NULL);
-
- return v;
+ return validate_test_init_internal(data, json_string, NULL);
}
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 3f6bc4d..8856f31 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -36,9 +36,27 @@ static void visitor_input_teardown(TestInputVisitorData *data,
}
}
-/* This is provided instead of a test setup function so that the JSON
- string used by the tests are kept in the test functions (and not
- int main()) */
+/* The various test_init functions are provided instead of a test setup
+ function so that the JSON string used by the tests are kept in the test
+ functions (and not in main()). */
+static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
+ const char *json_string,
+ va_list *ap)
+{
+ Visitor *v;
+
+ data->obj = qobject_from_jsonv(json_string, ap);
+ g_assert(data->obj);
+
+ data->qiv = qmp_input_visitor_new(data->obj);
+ g_assert(data->qiv);
+
+ v = qmp_input_get_visitor(data->qiv);
+ g_assert(v);
+
+ return v;
+}
+
static GCC_FMT_ATTR(2, 3)
Visitor *visitor_input_test_init(TestInputVisitorData *data,
const char *json_string, ...)
@@ -47,17 +65,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
va_list ap;
va_start(ap, json_string);
- data->obj = qobject_from_jsonv(json_string, &ap);
+ v = visitor_input_test_init_internal(data, json_string, &ap);
va_end(ap);
-
- g_assert(data->obj != NULL);
-
- data->qiv = qmp_input_visitor_new(data->obj);
- g_assert(data->qiv != NULL);
-
- v = qmp_input_get_visitor(data->qiv);
- g_assert(v != NULL);
-
return v;
}
@@ -71,19 +80,7 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
const char *json_string)
{
- Visitor *v;
-
- data->obj = qobject_from_json(json_string);
-
- g_assert(data->obj != NULL);
-
- data->qiv = qmp_input_visitor_new(data->obj);
- g_assert(data->qiv != NULL);
-
- v = qmp_input_get_visitor(data->qiv);
- g_assert(v != NULL);
-
- return v;
+ return visitor_input_test_init_internal(data, json_string, NULL);
}
static void test_visitor_in_int(TestInputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 05/12] qapi: Plug leaks in test-qmp-*
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (3 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 04/12] qapi: Share test_init code in test-qmp-input* Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 06/12] qapi: Simplify non-error testing " Markus Armbruster
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
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).
A final leak was in test_visitor_out_any(), which was reassigning
the qobj local variable to a subset of the overall structure
needing freeing; it did not result in a use-after-free, but
was not cleaning up all the qdict.
test-qmp-event and test-qmp-commands were already clean.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-6-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-input-strict.c | 9 +++++++++
tests/test-qmp-input-visitor.c | 41 +++++++----------------------------------
tests/test-qmp-output-visitor.c | 3 ++-
3 files changed, 18 insertions(+), 35 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 4837b31..821efe0 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -49,6 +49,8 @@ static Visitor *validate_test_init_internal(TestInputVisitorData *data,
{
Visitor *v;
+ validate_teardown(data, NULL);
+
data->obj = qobject_from_jsonv(json_string, ap);
g_assert(data->obj);
@@ -191,6 +193,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
visit_type_TestStruct(v, &p, NULL, &err);
g_assert(err);
+ error_free(err);
if (p) {
g_free(p->string);
}
@@ -208,6 +211,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);
}
@@ -222,6 +226,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);
}
@@ -237,6 +242,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);
}
@@ -251,6 +257,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);
}
@@ -266,6 +273,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);
}
@@ -280,6 +288,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 8856f31..8d49a97 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -45,6 +45,8 @@ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
{
Visitor *v;
+ visitor_input_teardown(data, NULL);
+
data->obj = qobject_from_jsonv(json_string, ap);
g_assert(data->obj);
@@ -174,12 +176,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;
}
@@ -202,12 +199,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)
{
@@ -223,17 +214,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->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,
@@ -343,14 +331,12 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
g_assert_cmpint(tmp->u.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_cmpstr(tmp->u.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);
@@ -358,7 +344,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,
@@ -381,7 +366,6 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
error_free(err);
err = NULL;
qapi_free_AltStrBool(asb);
- visitor_input_teardown(data, NULL);
/* FIXME: Order of alternate should not affect semantics; asn should
* parse the same as ans */
@@ -393,35 +377,30 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
error_free(err);
err = NULL;
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, ==, ALT_NUM_STR_KIND_N);
g_assert_cmpfloat(ans->u.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, ==, ALT_STR_INT_KIND_I);
g_assert_cmpint(asi->u.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->u.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->u.i, ==, 42);
qapi_free_AltNumInt(ani);
- visitor_input_teardown(data, NULL);
/* Parsing a double */
@@ -431,21 +410,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, ==, ALT_STR_NUM_KIND_N);
g_assert_cmpfloat(asn->u.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_cmpfloat(ans->u.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);
@@ -453,21 +429,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, ==, ALT_INT_NUM_KIND_N);
g_assert_cmpfloat(ain->u.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_cmpfloat(ani->u.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 9364843..8606111 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -391,6 +391,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);
@@ -411,7 +412,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,
@@ -463,6 +463,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);
}
static void test_visitor_out_empty(TestOutputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 06/12] qapi: Simplify non-error testing in test-qmp-*
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (4 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 05/12] qapi: Plug leaks in test-qmp-* Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup " Markus Armbruster
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
By using &error_abort, we can avoid a local err variable in
situations where we expect success. It also has the nice
effect that if the test breaks, the error message from
error_abort tends to be nicer than that of g_assert().
This patch has an additional bonus of fixing several call sites that
were passing &err to two different functions without checking it in
between. In general that is unsafe practice; because if the first
function sets an error, the second function could abort() if it tries to
set a different error. We got away with it because we were asserting
that err was NULL through the entire chain, but switching to
&error_abort avoids the questionable practice up front.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-7-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-input-strict.c | 31 +++++---------------
tests/test-qmp-input-visitor.c | 59 ++++++++++----------------------------
tests/test-qmp-output-visitor.c | 56 +++++++++---------------------------
tests/test-visitor-serialization.c | 42 +++++++++++----------------
4 files changed, 53 insertions(+), 135 deletions(-)
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 821efe0..d91539c 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -94,13 +94,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);
}
@@ -109,7 +107,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', "
@@ -117,8 +114,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);
}
@@ -126,13 +122,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);
}
@@ -141,12 +135,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);
}
@@ -155,7 +147,6 @@ static void test_validate_union_flat(TestInputVisitorData *data,
{
UserDefFlatUnion *tmp = NULL;
Visitor *v;
- Error *err = NULL;
v = validate_test_init(data,
"{ 'enum1': 'value1', "
@@ -163,8 +154,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);
}
@@ -173,12 +163,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);
}
@@ -296,16 +284,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 8d49a97..630a69f 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -89,13 +89,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);
}
@@ -120,14 +118,12 @@ static void test_visitor_in_int_overflow(TestInputVisitorData *data,
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);
}
@@ -135,13 +131,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);
}
@@ -149,13 +143,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);
@@ -164,7 +156,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;
@@ -173,8 +164,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);
}
}
@@ -184,13 +174,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");
@@ -203,7 +191,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', "
@@ -211,8 +198,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");
@@ -228,14 +214,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++) {
@@ -253,7 +237,6 @@ static void test_visitor_in_any(TestInputVisitorData *data,
const void *unused)
{
QObject *res = NULL;
- Error *err = NULL;
Visitor *v;
QInt *qint;
QBool *qbool;
@@ -262,16 +245,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");
@@ -296,7 +277,6 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data,
const void *unused)
{
Visitor *v;
- Error *err = NULL;
UserDefFlatUnion *tmp;
UserDefUnionBase *base;
@@ -306,8 +286,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);
@@ -448,7 +427,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("");
@@ -465,8 +443,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->type, ==, kind);
@@ -611,7 +588,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("");
@@ -628,8 +604,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->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
@@ -647,7 +622,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("");
@@ -663,8 +637,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->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
@@ -686,7 +659,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("");
@@ -702,8 +674,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->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 8606111..0164984 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;
@@ -225,8 +210,7 @@ static void test_visitor_out_struct_nested(TestOutputVisitorData *data,
ud2->dict1->dict3->userdef->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);
@@ -288,7 +272,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;
@@ -306,8 +289,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);
@@ -367,7 +349,6 @@ static void test_visitor_out_any(TestOutputVisitorData *data,
const void *unused)
{
QObject *qobj;
- Error *err = NULL;
QInt *qint;
QBool *qbool;
QString *qstring;
@@ -375,8 +356,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);
@@ -389,8 +369,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);
@@ -420,8 +399,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");
@@ -429,8 +406,7 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data,
tmp->integer = 41;
tmp->u.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);
@@ -449,14 +425,12 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
const void *unused)
{
QObject *arg;
- Error *err = NULL;
UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
tmp->type = USER_DEF_ALTERNATE_KIND_I;
tmp->u.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);
@@ -697,14 +671,12 @@ static void test_native_list(TestOutputVisitorData *data,
UserDefNativeListUnionKind kind)
{
UserDefNativeListUnion *cvalue = g_new0(UserDefNativeListUnion, 1);
- Error *err = NULL;
QObject *obj;
cvalue->type = 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->type);
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index c024e5e..9f67f9e 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -302,14 +302,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);
@@ -345,7 +344,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;
@@ -492,10 +490,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 */
@@ -652,10 +651,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);
}
@@ -665,13 +662,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);
@@ -687,14 +683,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);
@@ -709,7 +703,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;
@@ -720,11 +713,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] 15+ messages in thread* [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup in test-qmp-*
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (5 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 06/12] qapi: Simplify non-error testing " Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 18:06 ` Eric Blake
2015-11-09 17:46 ` [Qemu-devel] [PULL 08/12] qapi: More tests of alternate output Markus Armbruster
` (4 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
We have several tests that perform multiple sub-actions that are
expected to fail. Asserting that an error occurred, then clearing
it up to prepare for the next action, turned into enough
boilerplate that it was sometimes forgotten (for example, a number
of tests added to test-qmp-input-visitor.c in d88f5fd leaked err).
Worse, if an error is not reset to NULL, we risk invalidating
later use of that error (passing a non-NULL err into a function
is generally a bad idea). Encapsulate the boilerplate into a
single helper function error_free_or_abort(), and consistently
use it.
The new function is added into error.c for use everywhere,
although it is anticipated that testsuites will be the main
client.
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/qapi/error.h | 9 +++++++++
tests/test-qmp-commands.c | 3 +--
tests/test-qmp-input-strict.c | 21 +++++++--------------
tests/test-qmp-input-visitor.c | 26 +++++++-------------------
util/error.c | 7 +++++++
5 files changed, 31 insertions(+), 35 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index c69dddb..faad466 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -30,6 +30,10 @@
* Handle an error without reporting it (just for completeness):
* error_free(err);
*
+ * Assert than an expected error occurred, but clean it up without
+ * reporting it (primarily useful in testsuites):
+ * error_free_or_abort(&err);
+ *
* Pass an existing error to the caller:
* error_propagate(errp, err);
* where Error **errp is a parameter, by convention the last one.
@@ -190,6 +194,11 @@ Error *error_copy(const Error *err);
void error_free(Error *err);
/*
+ * Convenience function to assert that *@errp is set, then silently free it.
+ */
+void error_free_or_abort(Error **errp);
+
+/*
* Convenience function to error_report() and free @err.
*/
void error_report_err(Error *);
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index f23d8ea..888fb5f 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -225,8 +225,7 @@ static void test_dealloc_partial(void)
assert(ud2->dict1 == NULL);
/* confirm & release construction error */
- assert(err != NULL);
- error_free(err);
+ error_free_or_abort(&err);
/* tear down partial object */
qapi_free_UserDefTwo(ud2);
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index d91539c..f1c2e3b 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -180,8 +180,7 @@ static void test_validate_fail_struct(TestInputVisitorData *data,
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);
+ error_free_or_abort(&err);
if (p) {
g_free(p->string);
}
@@ -198,8 +197,7 @@ static void test_validate_fail_struct_nested(TestInputVisitorData *data,
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);
+ error_free_or_abort(&err);
qapi_free_UserDefTwo(udp);
}
@@ -213,8 +211,7 @@ static void test_validate_fail_list(TestInputVisitorData *data,
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);
+ error_free_or_abort(&err);
qapi_free_UserDefOneList(head);
}
@@ -229,8 +226,7 @@ static void test_validate_fail_union_native_list(TestInputVisitorData *data,
"{ 'type': 'integer', 'data' : [ 'string' ] }");
visit_type_UserDefNativeListUnion(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ error_free_or_abort(&err);
qapi_free_UserDefNativeListUnion(tmp);
}
@@ -244,8 +240,7 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
v = validate_test_init(data, "{ 'string': 'c', 'integer': 41, 'boolean': true }");
visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ error_free_or_abort(&err);
qapi_free_UserDefFlatUnion(tmp);
}
@@ -260,8 +255,7 @@ static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
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);
+ error_free_or_abort(&err);
qapi_free_UserDefFlatUnion2(tmp);
}
@@ -275,8 +269,7 @@ static void test_validate_fail_alternate(TestInputVisitorData *data,
v = validate_test_init(data, "3.14");
visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
+ error_free_or_abort(&err);
qapi_free_UserDefAlternate(tmp);
}
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 630a69f..ef5284d 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -111,8 +111,7 @@ 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);
+ error_free_or_abort(&err);
}
static void test_visitor_in_bool(TestInputVisitorData *data,
@@ -319,9 +318,7 @@ static void test_visitor_in_alternate(TestInputVisitorData *data,
v = visitor_input_test_init(data, "false");
visit_type_UserDefAlternate(v, &tmp, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
qapi_free_UserDefAlternate(tmp);
}
@@ -341,9 +338,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42");
visit_type_AltStrBool(v, &asb, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
qapi_free_AltStrBool(asb);
/* FIXME: Order of alternate should not affect semantics; asn should
@@ -352,9 +347,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
visit_type_AltStrNum(v, &asn, NULL, &err);
/* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
/* FIXME g_assert_cmpfloat(asn->u.n, ==, 42); */
- g_assert(err);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
qapi_free_AltStrNum(asn);
v = visitor_input_test_init(data, "42");
@@ -385,9 +378,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrBool(v, &asb, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
qapi_free_AltStrBool(asb);
v = visitor_input_test_init(data, "42.5");
@@ -404,9 +395,7 @@ static void test_visitor_in_alternate_number(TestInputVisitorData *data,
v = visitor_input_test_init(data, "42.5");
visit_type_AltStrInt(v, &asi, NULL, &err);
- g_assert(err);
- error_free(err);
- err = NULL;
+ error_free_or_abort(&err);
qapi_free_AltStrInt(asi);
v = visitor_input_test_init(data, "42.5");
@@ -713,12 +702,11 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
visit_type_TestStruct(v, &p, NULL, &err);
- g_assert(err);
+ error_free_or_abort(&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/util/error.c b/util/error.c
index 8b86490..80c89a2 100644
--- a/util/error.c
+++ b/util/error.c
@@ -220,6 +220,13 @@ void error_free(Error *err)
}
}
+void error_free_or_abort(Error **errp)
+{
+ assert(errp && *errp);
+ error_free(*errp);
+ *errp = NULL;
+}
+
void error_propagate(Error **dst_errp, Error *local_err)
{
if (!local_err) {
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup in test-qmp-*
2015-11-09 17:46 ` [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup " Markus Armbruster
@ 2015-11-09 18:06 ` Eric Blake
2015-11-10 7:36 ` Markus Armbruster
0 siblings, 1 reply; 15+ messages in thread
From: Eric Blake @ 2015-11-09 18:06 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]
On 11/09/2015 10:46 AM, Markus Armbruster wrote:
> From: Eric Blake <eblake@redhat.com>
>
> We have several tests that perform multiple sub-actions that are
> expected to fail. Asserting that an error occurred, then clearing
> it up to prepare for the next action, turned into enough
> boilerplate that it was sometimes forgotten (for example, a number
> of tests added to test-qmp-input-visitor.c in d88f5fd leaked err).
> Worse, if an error is not reset to NULL, we risk invalidating
> later use of that error (passing a non-NULL err into a function
> is generally a bad idea). Encapsulate the boilerplate into a
> single helper function error_free_or_abort(), and consistently
> use it.
>
> The new function is added into error.c for use everywhere,
> although it is anticipated that testsuites will be the main
> client.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> +++ b/include/qapi/error.h
> @@ -30,6 +30,10 @@
> * Handle an error without reporting it (just for completeness):
> * error_free(err);
> *
> + * Assert than an expected error occurred, but clean it up without
> + * reporting it (primarily useful in testsuites):
s/than/that/ (if the pull hasn't already gone through; otherwise it's a
trivial followup)
--
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] 15+ messages in thread
* Re: [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup in test-qmp-*
2015-11-09 18:06 ` Eric Blake
@ 2015-11-10 7:36 ` Markus Armbruster
0 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-10 7:36 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel
Eric Blake <eblake@redhat.com> writes:
> On 11/09/2015 10:46 AM, Markus Armbruster wrote:
>> From: Eric Blake <eblake@redhat.com>
>>
>> We have several tests that perform multiple sub-actions that are
>> expected to fail. Asserting that an error occurred, then clearing
>> it up to prepare for the next action, turned into enough
>> boilerplate that it was sometimes forgotten (for example, a number
>> of tests added to test-qmp-input-visitor.c in d88f5fd leaked err).
>> Worse, if an error is not reset to NULL, we risk invalidating
>> later use of that error (passing a non-NULL err into a function
>> is generally a bad idea). Encapsulate the boilerplate into a
>> single helper function error_free_or_abort(), and consistently
>> use it.
>>
>> The new function is added into error.c for use everywhere,
>> although it is anticipated that testsuites will be the main
>> client.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/include/qapi/error.h
>> @@ -30,6 +30,10 @@
>> * Handle an error without reporting it (just for completeness):
>> * error_free(err);
>> *
>> + * Assert than an expected error occurred, but clean it up without
>> + * reporting it (primarily useful in testsuites):
>
> s/than/that/ (if the pull hasn't already gone through; otherwise it's a
> trivial followup)
Fixed in PULL v2, thanks!
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PULL 08/12] qapi: More tests of alternate output
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (6 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 07/12] qapi: Simplify error cleanup " Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 09/12] qapi: Test failure in middle of array parse Markus Armbruster
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
The testsuite was only covering that we could output the 'int'
branch of an alternate (no additional allocation/cleanup required).
Add a test of the 'str' branch, to make sure that things still
work even when a branch involves allocation.
Update to modern style of g_new0() over g_malloc0() while
touching it.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-9-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-output-visitor.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 0164984..0d0c859 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -425,8 +425,9 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
const void *unused)
{
QObject *arg;
+ UserDefAlternate *tmp;
- UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
+ tmp = g_new0(UserDefAlternate, 1);
tmp->type = USER_DEF_ALTERNATE_KIND_I;
tmp->u.i = 42;
@@ -438,6 +439,19 @@ static void test_visitor_out_alternate(TestOutputVisitorData *data,
qapi_free_UserDefAlternate(tmp);
qobject_decref(arg);
+
+ tmp = g_new0(UserDefAlternate, 1);
+ tmp->type = USER_DEF_ALTERNATE_KIND_S;
+ tmp->u.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);
+ qobject_decref(arg);
}
static void test_visitor_out_empty(TestOutputVisitorData *data,
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 09/12] qapi: Test failure in middle of array parse
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (7 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 08/12] qapi: More tests of alternate output Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 10/12] qapi: More tests of input arrays Markus Armbruster
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
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.
There are more test cases throughout the test-qmp-input-* tests
that already deal with partial allocation; a later commit will
clean up all visit_type_FOO(), without marking all of the tests
with FIXME at this time.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-10-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi-visit.py | 4 ++++
tests/test-qmp-input-visitor.c | 10 +++++++++-
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index f40c3c7..3ef5c16 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -138,6 +138,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 ef5284d..aa02eec 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -698,8 +698,10 @@ static void test_visitor_in_errors(TestInputVisitorData *data,
TestStruct *p = NULL;
Error *err = NULL;
Visitor *v;
+ strList *q = NULL;
- v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', 'string': -42 }");
+ v = visitor_input_test_init(data, "{ 'integer': false, 'boolean': 'foo', "
+ "'string': -42 }");
visit_type_TestStruct(v, &p, NULL, &err);
error_free_or_abort(&err);
@@ -709,6 +711,12 @@ 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' ]");
+ visit_type_strList(v, &q, NULL, &err);
+ error_free_or_abort(&err);
+ assert(q);
+ qapi_free_strList(q);
}
int main(int argc, char **argv)
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PULL 10/12] qapi: More tests of input arrays
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (8 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 09/12] qapi: Test failure in middle of array parse Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 11/12] qapi: Provide nicer array names in introspection Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 12/12] qapi-introspect: Document lack of sorting Markus Armbruster
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
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.
I did not think it worth duplicating these additions to
test-qmp-input-strict; since all strict mode does is add
the ability to reject JSON input that has more keys than
what the visitor expects, yet the additions in this patch
error out earlier than that point regardless of whether
strict mode was requested.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-11-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/test-qmp-input-visitor.c | 52 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index aa02eec..d48ebdd 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -230,6 +230,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,
@@ -719,6 +725,50 @@ 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;
+ Error *err = NULL;
+
+ /* Make sure arrays and structs cannot be confused */
+
+ v = visitor_input_test_init(data, "[]");
+ visit_type_TestStruct(v, &p, NULL, &err);
+ error_free_or_abort(&err);
+ g_assert(!p);
+
+ v = visitor_input_test_init(data, "{}");
+ visit_type_strList(v, &q, NULL, &err);
+ error_free_or_abort(&err);
+ assert(!q);
+
+ /* Make sure primitives and struct cannot be confused */
+
+ v = visitor_input_test_init(data, "1");
+ visit_type_TestStruct(v, &p, NULL, &err);
+ error_free_or_abort(&err);
+ g_assert(!p);
+
+ v = visitor_input_test_init(data, "{}");
+ visit_type_int(v, &i, NULL, &err);
+ error_free_or_abort(&err);
+
+ /* Make sure primitives and arrays cannot be confused */
+
+ v = visitor_input_test_init(data, "1");
+ visit_type_strList(v, &q, NULL, &err);
+ error_free_or_abort(&err);
+ assert(!q);
+
+ v = visitor_input_test_init(data, "[]");
+ visit_type_int(v, &i, NULL, &err);
+ error_free_or_abort(&err);
+}
+
int main(int argc, char **argv)
{
TestInputVisitorData in_visitor_data;
@@ -751,6 +801,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] 15+ messages in thread* [Qemu-devel] [PULL 11/12] qapi: Provide nicer array names in introspection
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (9 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 10/12] qapi: More tests of input arrays Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
2015-11-09 17:46 ` [Qemu-devel] [PULL 12/12] qapi-introspect: Document lack of sorting Markus Armbruster
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
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 (just over 200 bytes), but it's in the noise (less than
0.3% of the overall 70k size of 'query-qmp-capabilities').
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-12-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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 4d8c2fc..ba29bc6 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -662,11 +662,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] 15+ messages in thread* [Qemu-devel] [PULL 12/12] qapi-introspect: Document lack of sorting
2015-11-09 17:46 [Qemu-devel] [PULL 00/12] QAPI patches Markus Armbruster
` (10 preceding siblings ...)
2015-11-09 17:46 ` [Qemu-devel] [PULL 11/12] qapi: Provide nicer array names in introspection Markus Armbruster
@ 2015-11-09 17:46 ` Markus Armbruster
11 siblings, 0 replies; 15+ messages in thread
From: Markus Armbruster @ 2015-11-09 17:46 UTC (permalink / raw)
To: qemu-devel
From: Eric Blake <eblake@redhat.com>
qapi-code-gen.txt already claims that types, commands, and
events share a common namespace; set this in stone by further
documenting that our introspection output will never have
collisions with the same name tied to more than one meta-type.
Our largest QMP enum currently has 125 values, our largest
object type has 27 members, and the mean for each is less than
10. These sizes are small enough that the per-element overhead
of O(log n) binary searching probably outweighs the speed
possible with direct O(n) linear searching (a better algorithm
with more overhead will only beat a leaner naive algorithm only
as you scale to larger input sizes).
Arguably, the overall SchemaInfo array could be sorted by name;
there, we currently have 531 entities, large enough for a binary
search to be faster than linear. However, remember that we have
mutually-recursive types, which means there is no topological
ordering that will allow clients to learn all information about
that type in a single linear pass; thus clients will want to do
random access over the data, and they will probably read the
introspection output into a hashtable for O(1) lookup rather
than O(log n) binary searching, at which point, pre-sorting our
introspection output doesn't help the client.
It doesn't help that sorting can be subjective if you introduce
locales into the mix (I'm not experienced enough with Python
to know for sure, but at least it looks like it defaults to
sorting in the C locale even when run under a different locale).
And while our current introspection output is deterministic
(because we visit entities in a sorted order), we may want
to change that order in the future (such as using OrderedDict
to stick to .json declaration order).
For these reasons, we simply document that clients should not
rely on any particular order of items in introspection output.
And since it is now a documented part of the contract, we have
the freedom to later rearrange output if needed, without
worrying about breaking well-written clients that were already
aware that they had to do linear searches.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1446791754-23823-13-git-send-email-eblake@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
docs/qapi-code-gen.txt | 19 +++++++++++++++----
qapi/introspect.json | 17 ++++++++++++-----
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index ba29bc6..f9fa6f3 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -516,6 +516,10 @@ 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 throughout the entire array
+to learn more about that name, but is at least guaranteed that there
+will be no collisions between type, command, and event names.
However, the SchemaInfo can't reflect all the rules and restrictions
that apply to QMP. It's interface introspection (figuring out what's
@@ -596,7 +600,9 @@ 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 in no particular order; clients
+must search the entire object when learning whether a particular
+member is supported.
Example: the SchemaInfo for MyType from section Struct types
@@ -610,7 +616,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 in no particular order, and is not guaranteed to
+list cases in the same order as the corresponding "tag" enum type.
Example: the SchemaInfo for flat union BlockdevOptions from section
Union types
@@ -651,7 +659,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
@@ -673,7 +682,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 no particular
+order; clients must search the entire enum when learning whether a
+particular value is supported.
Example: the SchemaInfo for MyEnum from section Enumeration types
diff --git a/qapi/introspect.json b/qapi/introspect.json
index cc50dc6..e7c4c3e 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -25,6 +25,10 @@
# Returns: array of @SchemaInfo, where each element describes an
# entity in the ABI: command, event, type, ...
#
+# The order of the various SchemaInfo is unspecified; however, all
+# names are guaranteed to be unique (no name will be duplicated with
+# different meta-types).
+#
# 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 +82,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 they are still guaranteed unique
+# regardless of @meta-type.
#
# All references to other SchemaInfo are by name.
#
@@ -130,7 +135,7 @@
#
# Additional SchemaInfo members for meta-type 'enum'.
#
-# @values: the enumeration type's values.
+# @values: the enumeration type's values, in no particular order.
#
# Values of this type are JSON string on the wire.
#
@@ -158,14 +163,16 @@
#
# Additional SchemaInfo members for meta-type 'object'.
#
-# @members: the object type's (non-variant) members.
+# @members: the object type's (non-variant) members, in no particular order.
#
# @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 in no particular order,
+# and may even differ from the order of the values of the
+# enum type of the @tag.
#
# Values of this type are JSON object on the wire.
#
@@ -219,7 +226,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.
#
--
2.4.3
^ permalink raw reply related [flat|nested] 15+ messages in thread