Re: [PATCH v3 05/34] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
Marc-André Lureau writes: > Hi > > On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster wrote: >> >> Checking the value of qmp_dispatch() is repetitive. Factor out >> helpers do_qmp_dispatch() and do_qmp_dispatch_error(). Without this, >> the next commit would make things even more repetitive. >> >> Signed-off-by: Markus Armbruster >> --- >> tests/test-qmp-cmds.c | 72 +-- >> 1 file changed, 35 insertions(+), 37 deletions(-) > > ASAN is unhappy: > > = > ==1870336==ERROR: LeakSanitizer: detected memory leaks > > Indirect leak of 4120 byte(s) in 1 object(s) allocated from: > #0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56) > #1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0) > #2 0x560213f56dbb in test_dispatch_cmd_io > /home/elmarco/src/qemu/tests/test-qmp-cmds.c:238 > #3 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d) [...] Thanks! Fixup to be squashed in here, with revert to be squashed into PATCH 07: diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index fb18475c7e..b31064b064 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -279,8 +279,6 @@ static void test_dispatch_cmd_io(void) g_assert(qnum_get_try_int(ret3, )); g_assert_cmpint(val, ==, 66); qobject_unref(ret3); - -qobject_unref(req); } /* test generated dealloc functions for generated types */
Re: [PATCH v3 05/34] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
On 3/15/20 9:46 AM, Markus Armbruster wrote:
Checking the value of qmp_dispatch() is repetitive. Factor out
helpers do_qmp_dispatch() and do_qmp_dispatch_error(). Without this,
the next commit would make things even more repetitive.
Signed-off-by: Markus Armbruster
---
tests/test-qmp-cmds.c | 72 +--
1 file changed, 35 insertions(+), 37 deletions(-)
+
+static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls)
+{
+QDict *resp;
+
+resp = qmp_dispatch(_commands, QOBJECT(req), allow_oob);
+g_assert(resp && qdict_haskey(resp, "error"));
+
+qobject_unref(resp);
+}
No checking of cls? Or is that what you hint at as coming later?
At any rate, the refactoring is sane.
Reviewed-by: Eric Blake
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Re: [PATCH v3 05/34] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
Hi
On Sun, Mar 15, 2020 at 3:48 PM Markus Armbruster wrote:
>
> Checking the value of qmp_dispatch() is repetitive. Factor out
> helpers do_qmp_dispatch() and do_qmp_dispatch_error(). Without this,
> the next commit would make things even more repetitive.
>
> Signed-off-by: Markus Armbruster
> ---
> tests/test-qmp-cmds.c | 72 +--
> 1 file changed, 35 insertions(+), 37 deletions(-)
ASAN is unhappy:
=
==1870336==ERROR: LeakSanitizer: detected memory leaks
Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56)
#1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0)
#2 0x560213f56dbb in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:238
#3 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56)
#1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0)
#2 0x560213f56dd3 in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:240
#3 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 55 byte(s) in 2 object(s) allocated from:
#0 0x7fcdc9b8bc58 in __interceptor_malloc (/lib64/libasan.so.5+0x10dc58)
#1 0x7fcdc998e358 in g_malloc (/lib64/libglib-2.0.so.0+0x57358)
#2 0x560213fbf065 in qstring_from_str
/home/elmarco/src/qemu/qobject/qstring.c:66
#3 0x560213fc151a in qdict_put_str
/home/elmarco/src/qemu/qobject/qdict.c:143
#4 0x560213f5747f in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:276
#5 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56)
#1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0)
#2 0x560213fc0ee4 in qdict_put_obj
/home/elmarco/src/qemu/qobject/qdict.c:125
#3 0x560213f56fca in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:254
#4 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56)
#1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0)
#2 0x560213fc0ee4 in qdict_put_obj
/home/elmarco/src/qemu/qobject/qdict.c:125
#3 0x560213fc147e in qdict_put_int
/home/elmarco/src/qemu/qobject/qdict.c:133
#4 0x560213f573ea in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:274
#5 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8bc58 in __interceptor_malloc (/lib64/libasan.so.5+0x10dc58)
#1 0x7fcdc998e358 in g_malloc (/lib64/libglib-2.0.so.0+0x57358)
#2 0x560213fc1428 in qdict_put_int
/home/elmarco/src/qemu/qobject/qdict.c:133
#3 0x560213f573ea in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:274
#4 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 32 byte(s) in 1 object(s) allocated from:
#0 0x7fcdc9b8be56 in __interceptor_calloc (/lib64/libasan.so.5+0x10de56)
#1 0x7fcdc998e3b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x573b0)
#2 0x560213fc0ee4 in qdict_put_obj
/home/elmarco/src/qemu/qobject/qdict.c:125
#3 0x560213fc1570 in qdict_put_str
/home/elmarco/src/qemu/qobject/qdict.c:143
#4 0x560213f56fe7 in test_dispatch_cmd_io
/home/elmarco/src/qemu/tests/test-qmp-cmds.c:255
#5 0x7fcdc99b0a8d (/lib64/libglib-2.0.so.0+0x79a8d)
Indirect leak of 20 byte(s) in 3 object(s) allocated from:
#0 0x7fcdc9b8bc58 in __interceptor_malloc (/lib64/libasan.so.5+0x10dc58)
#1 0x7fcdc998e358 in g_malloc (/lib64/libglib-2.0.so.0+0x57358)
>
> diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
> index 79507d9e54..b31064b064 100644
> --- a/tests/test-qmp-cmds.c
> +++ b/tests/test-qmp-cmds.c
> @@ -145,34 +145,55 @@ __org_qemu_x_Union1
> *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
> }
>
>
> +static QObject *do_qmp_dispatch(QDict *req, bool allow_oob)
> +{
> +QDict *resp;
> +QObject *ret;
> +
> +resp = qmp_dispatch(_commands, QOBJECT(req), allow_oob);
> +g_assert(resp && !qdict_haskey(resp, "error"));
> +ret = qdict_get(resp, "return");
> +g_assert(ret);
> +
> +qobject_ref(ret);
> +qobject_unref(resp);
> +return ret;
> +}
> +
> +static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls)
> +{
> +QDict *resp;
> +
> +resp = qmp_dispatch(_commands, QOBJECT(req), allow_oob);
> +g_assert(resp && qdict_haskey(resp, "error"));
> +
> +qobject_unref(resp);
> +}
> +
> /* test commands with no input and no return value */
> static void test_dispatch_cmd(void)
> {
> QDict *req = qdict_new();
>
[PATCH v3 05/34] tests/test-qmp-cmds: Factor out qmp_dispatch() test helpers
Checking the value of qmp_dispatch() is repetitive. Factor out
helpers do_qmp_dispatch() and do_qmp_dispatch_error(). Without this,
the next commit would make things even more repetitive.
Signed-off-by: Markus Armbruster
---
tests/test-qmp-cmds.c | 72 +--
1 file changed, 35 insertions(+), 37 deletions(-)
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 79507d9e54..b31064b064 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -145,34 +145,55 @@ __org_qemu_x_Union1
*qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
}
+static QObject *do_qmp_dispatch(QDict *req, bool allow_oob)
+{
+QDict *resp;
+QObject *ret;
+
+resp = qmp_dispatch(_commands, QOBJECT(req), allow_oob);
+g_assert(resp && !qdict_haskey(resp, "error"));
+ret = qdict_get(resp, "return");
+g_assert(ret);
+
+qobject_ref(ret);
+qobject_unref(resp);
+return ret;
+}
+
+static void do_qmp_dispatch_error(QDict *req, bool allow_oob, ErrorClass cls)
+{
+QDict *resp;
+
+resp = qmp_dispatch(_commands, QOBJECT(req), allow_oob);
+g_assert(resp && qdict_haskey(resp, "error"));
+
+qobject_unref(resp);
+}
+
/* test commands with no input and no return value */
static void test_dispatch_cmd(void)
{
QDict *req = qdict_new();
-QDict *resp;
+QObject *ret;
qdict_put_str(req, "execute", "user_def_cmd");
-resp = qmp_dispatch(_commands, QOBJECT(req), false);
-assert(resp != NULL);
-assert(!qdict_haskey(resp, "error"));
+ret = do_qmp_dispatch(req, false);
-qobject_unref(resp);
+qobject_unref(ret);
qobject_unref(req);
}
static void test_dispatch_cmd_oob(void)
{
QDict *req = qdict_new();
-QDict *resp;
+QObject *ret;
qdict_put_str(req, "exec-oob", "test-flags-command");
-resp = qmp_dispatch(_commands, QOBJECT(req), true);
-assert(resp != NULL);
-assert(!qdict_haskey(resp, "error"));
+ret = do_qmp_dispatch(req, true);
-qobject_unref(resp);
+qobject_unref(ret);
qobject_unref(req);
}
@@ -181,15 +202,11 @@ static void test_dispatch_cmd_failure(void)
{
QDict *req = qdict_new();
QDict *args = qdict_new();
-QDict *resp;
qdict_put_str(req, "execute", "user_def_cmd2");
-resp = qmp_dispatch(_commands, QOBJECT(req), false);
-assert(resp != NULL);
-assert(qdict_haskey(resp, "error"));
+do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR);
-qobject_unref(resp);
qobject_unref(req);
/* check that with extra arguments it throws an error */
@@ -199,11 +216,8 @@ static void test_dispatch_cmd_failure(void)
qdict_put_str(req, "execute", "user_def_cmd");
-resp = qmp_dispatch(_commands, QOBJECT(req), false);
-assert(resp != NULL);
-assert(qdict_haskey(resp, "error"));
+do_qmp_dispatch_error(req, false, ERROR_CLASS_GENERIC_ERROR);
-qobject_unref(resp);
qobject_unref(req);
}
@@ -218,20 +232,6 @@ static void test_dispatch_cmd_success_response(void)
qobject_unref(req);
}
-static QObject *test_qmp_dispatch(QDict *req)
-{
-QDict *resp;
-QObject *ret;
-
-resp = qmp_dispatch(_commands, QOBJECT(req), false);
-assert(resp && !qdict_haskey(resp, "error"));
-ret = qdict_get(resp, "return");
-assert(ret);
-qobject_ref(ret);
-qobject_unref(resp);
-return ret;
-}
-
/* test commands that involve both input parameters and return values */
static void test_dispatch_cmd_io(void)
{
@@ -254,7 +254,7 @@ static void test_dispatch_cmd_io(void)
qdict_put(req, "arguments", args);
qdict_put_str(req, "execute", "user_def_cmd2");
-ret = qobject_to(QDict, test_qmp_dispatch(req));
+ret = qobject_to(QDict, do_qmp_dispatch(req, false));
assert(!strcmp(qdict_get_str(ret, "string0"), "blah1"));
ret_dict = qdict_get_qdict(ret, "dict1");
@@ -275,12 +275,10 @@ static void test_dispatch_cmd_io(void)
qdict_put(req, "arguments", args3);
qdict_put_str(req, "execute", "guest-get-time");
-ret3 = qobject_to(QNum, test_qmp_dispatch(req));
+ret3 = qobject_to(QNum, do_qmp_dispatch(req, false));
g_assert(qnum_get_try_int(ret3, ));
g_assert_cmpint(val, ==, 66);
qobject_unref(ret3);
-
-qobject_unref(req);
}
/* test generated dealloc functions for generated types */
--
2.21.1
