On 09/12/2017 05:14 AM, Thomas Huth wrote: > 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?
D'oh. Yes, I do. >> + 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); Sure. I debated about it. If we ever do more than just create/destroy, then having the separate lines makes it easier to stick the actual test in between the two lines, so I'll avoid my abbreviation and go with the longer form on respin. >> 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... Passing void* through varargs, with the intent of the receiver parsing it as char*, is technically undefined in C. I don't know if gcc warns, but I'm also worried that clang might warn. I prefer to err on the side of defined behavior in this case, even though it annoyingly requires a temporary variable. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature