Re: [Qemu-devel] [PATCH v4 14/22] libqtest: Separate qmp_discard_response() from command

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 08/09/2017 10:15 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> Upcoming patches will be adding new convenience methods for
>>> constructing QMP commands.  But making every variation of sending
>>> support every variation of response handling becomes unwieldy;
>>> it's easier to specify that discarding a JSON response is
>>> unassociated with sending the command, where qmp_async() already
>>> fits the bill for sending a command without tying up a reference
>>> to the response.
>>>
>>> Doing this renders qtest_qmp[v]_discard_response() unused.
>>>
>>> Bonus: gets rid of a non-literal format string, which is a step
>>> towards compile-time format string checking without triggering
>>> -Wformat-nonliteral.
>> 
>> Where?  (I'm feeling lazy today)
>> 
>
> Sure:
>
>> 
>> +++ b/tests/ide-test.c
>> @@ -624,7 +624,6 @@ static void test_retry_flush(const char *machine)
>>      QPCIDevice *dev;
>>      QPCIBar bmdma_bar, ide_bar;
>>      uint8_t data;
>> -    const char *s;
>> 
>>      prepare_blkdebug_script(debug_path, "flush_to_disk");
>> 
>> @@ -652,8 +651,8 @@ static void test_retry_flush(const char *machine)
>>      qmp_eventwait("STOP");
>> 
>>      /* Complete the command */
>> -    s = "{'execute':'cont' }";
>> -    qmp_discard_response(s);
>> +    qmp_async("{'execute':'cont' }");
>> +    qmp_discard_response();
>
> Yes, I can update the commit message to focus in on it more precisely.

Not enabled by this patch, just cleanup while there.  Recommend to make
that clearer in the commit message.

Aside: I wonder what goes through a developer's head when he writes such
code.  "Too terse, let me splice in a variable" seems unlikely.

[...]