Re: [Qemu-devel] [PULL 12/25] tests: Clean up string interpolation into QMP input (simple cases)

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 08/16/2018 03:36 AM, Markus Armbruster wrote:
>> When you build QMP input manually like this
>>
>>      cmd = g_strdup_printf("{ 'execute': 'migrate',"
>>                            "'arguments': { 'uri': '%s' } }",
>>                            uri);
>>      rsp = qmp(cmd);
>>      g_free(cmd);
>>
>> you're responsible for escaping the interpolated values for JSON.  Not
>> done here, and therefore works only for sufficiently nice @uri.  For
>> instance, if @uri contained a single "'", qobject_from_vjsonf_nofail()
>> would abort.  A sufficiently nasty @uri could even inject unwanted
>> members into the arguments object.
>>
>> Leaving interpolation into JSON to qmp() is more robust:
>>
>>      rsp = qmp("{ 'execute': 'migrate', 'arguments': { 'uri': %s } }", uri);
>>
>> It's also more concise.
>>
>> Clean up the simple cases where we interpolate exactly a JSON value.
>>
>> Bonus: gets rid of non-literal format strings.  A step towards
>> compile-time format string checking without triggering
>> -Wformat-nonliteral.
>>
>
>> +++ b/tests/test-qga.c
>
>> @@ -446,10 +443,10 @@ static void test_qga_file_ops(gconstpointer fix)
>>         enc = g_base64_encode(helloworld, sizeof(helloworld));
>>       /* write */
>> -    cmd = g_strdup_printf("{'execute': 'guest-file-write',"
>> -                          " 'arguments': { 'handle': %" PRId64 ","
>> -                          " 'buf-b64': '%s' } }", id, enc);
>> -    ret = qmp_fd(fixture->fd, cmd);
>> +    ret = qmp_fd(fixture->fd,
>> +                 "{'execute': 'guest-file-write',"
>> +                 " 'arguments': { 'handle': %" PRId64 ", 'buf-b64': %s } }",
>> +                 id, enc);
>
> This is a temporary regression of commit 1792d7d0, which affects test
> execution on Mac (with its weird %qd expansion for
> PRId64). Fortunately, you later fix it in commit 41, so I won't be
> upset if the pull request goes through without a v2 that avoids 'git
> bisect' breaking on Mac.

Sorry about the accidental revert of your patch.  I rely on Peter's
testing to cover Mac.

This patch became commit 015715f554f, merged in commit c542a9f9794 on
August 16.  The "later fix" is commit 53a0d616fec, merged in commit
cc9821fa9ac on August 25.