Eric Blake <ebl...@redhat.com> writes:
> On 07/27/2018 10:13 AM, Markus Armbruster wrote:
>> Commit b21373d0713 copied wait_command() from tests/migration-test.c
>> to tests/tpm-util.c. Replace both copies by new libqtest helper
>> qtest_qmp_receive_success(). Also use it to simplify
>> qtest_qmp_device_del().
>>
>> Bonus: gets rid of a non-literal format string. A step towards
>> compile-time format string checking without triggering
>> -Wformat-nonliteral.
>
> Where? [1]
>
>>
>> Cc: Thomas Huth <th...@redhat.com>
>> Cc: Juan Quintela <quint...@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilb...@redhat.com>
>> Cc: Stefan Berger <stef...@linux.vnet.ibm.com>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> Reviewed-by: Juan Quintela <quint...@redhat.com>
>> Reviewed-by: Stefan Berger <stef...@linux.vnet.ibm.com>
>> ---
>
>> +++ b/tests/libqtest.c
>> @@ -1029,6 +1029,35 @@ void qtest_cb_for_every_machine(void (*cb)(const char
>> *machine))
>> qobject_unref(response);
>> }
>> +QDict *qtest_qmp_receive_success(QTestState *s,
>> + void (*event_cb)(void *opaque,
>> + const char *event,
>> + QDict *data),
>> + void *opaque)
>> +{
>
> I like the new signature!
>
>> +++ b/tests/migration-test.c
>
>> /*
>> * Events can get in the way of responses we are actually waiting for.
>> */
>> static QDict *wait_command(QTestState *who, const char *command)
>> {
>
> [1] This is still taking a format non-literal command...
>
>> - const char *event_string;
>> - QDict *response, *ret;
>> -
>> - response = qtest_qmp(who, command);
> ...
>> + qtest_qmp_send(who, command);
>
> and is passing it on through.
No worse than before. Will be fixed in the next two patches.
>> + return qtest_qmp_receive_success(who, stop_cb, NULL);
>> }
>>
>
>> +++ b/tests/tpm-util.c
>
>> -/*
>> - * Events can get in the way of responses we are actually waiting for.
>> - */
>> -static QDict *tpm_util_wait_command(QTestState *who, const char *command)
>> -{
>
> Maybe you were counting this instance,
>
- const char *event_string;
- QDict *response;
-
- response = qtest_qmp(who, command);
/work/armbru/qemu/tests/tpm-util.c: In function ‘tpm_util_wait_command’:
/work/armbru/qemu/tests/tpm-util.c:258:5: warning: format not a string literal
and no format arguments [-Wformat-security]
response = qtest_qmp(who, command);
^~~~~~~~
-
- while (qdict_haskey(response, "event")) {
- /* OK, it was an event */
- event_string = qdict_get_str(response, "event");
- if (!strcmp(event_string, "STOP")) {
- got_stop = true;
- }
- qobject_unref(response);
- response = qtest_qmp_receive(who);
- }
- return response;
-}
>> void tpm_util_wait_for_migration_complete(QTestState *who)
>> {
>> while (true) {
>> - QDict *rsp, *rsp_return;
>> + QDict *rsp_return;
>> bool completed;
>> const char *status;
>> - rsp = tpm_util_wait_command(who, "{ 'execute':
>> 'query-migrate' }");
>> - rsp_return = qdict_get_qdict(rsp, "return");
>> + qtest_qmp_send(who, "{ 'execute': 'query-migrate' }");
>
> where you did indeed drop a format non-literal?
> But on the assumption that there's more cleanups later in the series,
> this is indeed incrementally better, so
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
