Message ID | 20180712111221.20326-5-armbru@redhat.com |
---|---|
State | New |
Headers | show |
Series | tests: Compile-time format string checking for libqtest.h | expand |
On 12.07.2018 13:12, Markus Armbruster wrote: > qtest_init() still uses the qtest_qmp_discard_response(s, "") hack to > receive the greeting, even though we have qtest_qmp_receive() since > commit 66e0c7b187e. Put it to use. > > Bonus: gets rid of an empty format string. A step towards > compile-time format string checking without triggering > -Wformat-zero-length. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/libqtest.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/libqtest.c b/tests/libqtest.c > index 071d7eb7b1..c2c08a890c 100644 > --- a/tests/libqtest.c > +++ b/tests/libqtest.c > @@ -249,9 +249,11 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, > QTestState *qtest_init(const char *extra_args) > { > QTestState *s = qtest_init_without_qmp_handshake(false, extra_args); > + QDict *greeting; > > /* Read the QMP greeting and then do the handshake */ > - qtest_qmp_discard_response(s, ""); > + greeting = qtest_qmp_receive(s); > + qobject_unref(greeting); > qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); > > return s; I wonder whether we should actually check the greeting for some expected information? Anyway, that's something for later, and not related to your patch. Reviewed-by: Thomas Huth <thuth@redhat.com>
Thomas Huth <thuth@redhat.com> writes: > On 12.07.2018 13:12, Markus Armbruster wrote: >> qtest_init() still uses the qtest_qmp_discard_response(s, "") hack to >> receive the greeting, even though we have qtest_qmp_receive() since >> commit 66e0c7b187e. Put it to use. >> >> Bonus: gets rid of an empty format string. A step towards >> compile-time format string checking without triggering >> -Wformat-zero-length. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/libqtest.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tests/libqtest.c b/tests/libqtest.c >> index 071d7eb7b1..c2c08a890c 100644 >> --- a/tests/libqtest.c >> +++ b/tests/libqtest.c >> @@ -249,9 +249,11 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, >> QTestState *qtest_init(const char *extra_args) >> { >> QTestState *s = qtest_init_without_qmp_handshake(false, extra_args); >> + QDict *greeting; >> >> /* Read the QMP greeting and then do the handshake */ >> - qtest_qmp_discard_response(s, ""); >> + greeting = qtest_qmp_receive(s); >> + qobject_unref(greeting); >> qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); >> >> return s; > > I wonder whether we should actually check the greeting for some expected > information? Anyway, that's something for later, and not related to your > patch. This kind of sloppy testing is quite common. Checking test results manually in code maximizes flexibility. It also maximizes temptation to cut corners, because doing a complete job is so tedious. One reason I prefer "normalize and diff against expected results". The closest we got to support for that way of testing is qlit_equal_qobject(). Digression: QOM also maximizes flexibility by doing stuff in code rather than data. The two go back to the same stratum of QEMU development. Hardly coincidence, in my opinion. We've since extended QOM to support doing more in data, but there's still plenty of old code doing it in code, and plenty of new code following that old code's lead. Recovering from such damage is hard work. > Reviewed-by: Thomas Huth <thuth@redhat.com> Thanks!
diff --git a/tests/libqtest.c b/tests/libqtest.c index 071d7eb7b1..c2c08a890c 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -249,9 +249,11 @@ QTestState *qtest_init_without_qmp_handshake(bool use_oob, QTestState *qtest_init(const char *extra_args) { QTestState *s = qtest_init_without_qmp_handshake(false, extra_args); + QDict *greeting; /* Read the QMP greeting and then do the handshake */ - qtest_qmp_discard_response(s, ""); + greeting = qtest_qmp_receive(s); + qobject_unref(greeting); qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }"); return s;
qtest_init() still uses the qtest_qmp_discard_response(s, "") hack to receive the greeting, even though we have qtest_qmp_receive() since commit 66e0c7b187e. Put it to use. Bonus: gets rid of an empty format string. A step towards compile-time format string checking without triggering -Wformat-zero-length. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/libqtest.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)