[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 28/38] libqtest: Add qtest_[v]startf()
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH v7 28/38] libqtest: Add qtest_[v]startf() |
Date: |
Tue, 12 Sep 2017 12:14:52 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
On 11.09.2017 19:20, Eric Blake wrote:
> We have several callers that were formatting the argument strings
> themselves; consolidate this effort by adding new convenience
> functions directly in libqtest, and update all call-sites that
> can benefit from it.
[...]
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index e8c2e11817..b535d7768f 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -245,6 +245,27 @@ QTestState *qtest_start(const char *extra_args)
> return global_qtest = s;
> }
>
> +QTestState *qtest_vstartf(const char *fmt, va_list ap)
> +{
> + char *args = g_strdup_vprintf(fmt, ap);
> + QTestState *s;
> +
> + s = qtest_start(args);
> + global_qtest = NULL;
Don't you need a g_free(args) here?
> + return s;
> +}
> +
> +QTestState *qtest_startf(const char *fmt, ...)
> +{
> + va_list ap;
> + QTestState *s;
> +
> + va_start(ap, fmt);
> + s = qtest_vstartf(fmt, ap);
> + va_end(ap);
> + return s;
> +}
[...]
> diff --git a/tests/e1000-test.c b/tests/e1000-test.c
> index 0c5fcdcc44..12bc526ad6 100644
> --- a/tests/e1000-test.c
> +++ b/tests/e1000-test.c
> @@ -14,16 +14,8 @@
> static void test_device(gconstpointer data)
> {
> const char *model = data;
> - QTestState *s;
> - char *args;
>
> - args = g_strdup_printf("-device %s", model);
> - s = qtest_start(args);
> -
> - if (s) {
> - qtest_quit(s);
> - }
> - g_free(args);
> + qtest_quit(qtest_startf("-device %s", model));
Just my personal taste, but I think I'd be nicer to keep this on
separate lines:
QTestState *s;
s = qtest_startf("-device %s", model);
qtest_quit(s);
> }
[...]
> diff --git a/tests/eepro100-test.c b/tests/eepro100-test.c
> index bdc8a67d57..fc9ea84d66 100644
> --- a/tests/eepro100-test.c
> +++ b/tests/eepro100-test.c
> @@ -13,18 +13,9 @@
> static void test_device(gconstpointer data)
> {
> const char *model = data;
> - QTestState *s;
> - char *args;
> -
> - args = g_strdup_printf("-device %s", model);
> - s = qtest_start(args);
>
> /* Tests only initialization so far. TODO: Implement functional tests */
> -
> - if (s) {
> - qtest_quit(s);
> - }
> - g_free(args);
> + qtest_quit(qtest_startf("-device %s", model));
> }
dito
[...]
> diff --git a/tests/numa-test.c b/tests/numa-test.c
> index fa21d26935..e2f6c68be8 100644
> --- a/tests/numa-test.c
> +++ b/tests/numa-test.c
> @@ -12,20 +12,14 @@
> #include "qemu/osdep.h"
> #include "libqtest.h"
>
> -static char *make_cli(const char *generic_cli, const char *test_cli)
> -{
> - return g_strdup_printf("%s %s", generic_cli ? generic_cli : "",
> test_cli);
> -}
> -
> static void test_mon_explicit(const void *data)
> {
> char *s;
> - char *cli;
> + const char *args = data;
>
> - cli = make_cli(data, "-smp 8 "
> - "-numa node,nodeid=0,cpus=0-3 "
> - "-numa node,nodeid=1,cpus=4-7 ");
> - qtest_start(cli);
> + global_qtest = qtest_startf("%s -smp 8 "
> + "-numa node,nodeid=0,cpus=0-3 "
> + "-numa node,nodeid=1,cpus=4-7 ", args);
>
> s = hmp("info numa");
> g_assert(strstr(s, "node 0 cpus: 0 1 2 3"));
> @@ -33,16 +27,14 @@ static void test_mon_explicit(const void *data)
> g_free(s);
>
> qtest_quit(global_qtest);
> - g_free(cli);
> }
>
> static void test_mon_default(const void *data)
> {
> char *s;
> - char *cli;
> + const char *args = data;
>
> - cli = make_cli(data, "-smp 8 -numa node -numa node");
> - qtest_start(cli);
> + global_qtest = qtest_startf("%s -smp 8 -numa node -numa node", args);
>
> s = hmp("info numa");
> g_assert(strstr(s, "node 0 cpus: 0 2 4 6"));
> @@ -50,18 +42,16 @@ static void test_mon_default(const void *data)
> g_free(s);
>
> qtest_quit(global_qtest);
> - g_free(cli);
> }
>
> static void test_mon_partial(const void *data)
> {
> char *s;
> - char *cli;
> + const char *args = data;
>
> - cli = make_cli(data, "-smp 8 "
> - "-numa node,nodeid=0,cpus=0-1 "
> - "-numa node,nodeid=1,cpus=4-5 ");
> - qtest_start(cli);
> + global_qtest = qtest_startf("%s -smp 8 "
> + "-numa node,nodeid=0,cpus=0-1 "
> + "-numa node,nodeid=1,cpus=4-5 ", args);
Does GCC emit a warning if you'd used data here directly? Otherwise I
think you could simply do this without the local args variable...
Thomas
- [Qemu-devel] [PATCH v7 25/38] wdt_ib700-test: Drop dependence on global_qtest, (continued)
- [Qemu-devel] [PATCH v7 25/38] wdt_ib700-test: Drop dependence on global_qtest, Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 27/38] libqtest: Swap order of qtest_init() and qtest_start(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 30/38] qtest: Avoid passing raw strings through hmp(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 26/38] libqtest: Merge qtest_end() into qtest_quit(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 28/38] libqtest: Add qtest_[v]startf(), Eric Blake, 2017/09/11
- Re: [Qemu-devel] [PATCH v7 28/38] libqtest: Add qtest_[v]startf(),
Thomas Huth <=
- [Qemu-devel] [PATCH v7 29/38] libqtest: Merge qtest_init() into qtest_start(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 32/38] libqtest: Merge qtest_irq*() with irq*(), Eric Blake, 2017/09/11
- [Qemu-devel] [PATCH v7 31/38] libqtest: Merge qtest_clock_*() with clock_*(), Eric Blake, 2017/09/11