[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/9] tests: Clean up string interpolation into Q
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 4/9] tests: Clean up string interpolation into QMP input (simple cases) |
Date: |
Fri, 21 Jul 2017 09:39:23 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/21/2017 08:53 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_jsonv() would
> fail, qmp_fd_sendv() would misinterpret the failure as empty input and
> do nothing, and the test would hang waiting for a response that never
> comes.
>
> 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.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> + qmp_fd_send(fixture->fd,
> + "\xff{'execute': 'guest-sync-delimited',"
> + " 'arguments': {'id': %u } }",
Ouch. Per 2/9:
* json-lexer.c (only understands '%((l|ll|I64)?d|[ipsf])').
That will need to be %d now. Or, more likely, we need to update the
comments in json-lexer.c and in 2/9 (/me goes to read json-lexer.c...)
/* escape */
[IN_ESCAPE_LL] = {
['d'] = JSON_ESCAPE,
['u'] = JSON_ESCAPE,
},
...
[IN_ESCAPE] = {
['d'] = JSON_ESCAPE,
['i'] = JSON_ESCAPE,
['p'] = JSON_ESCAPE,
['s'] = JSON_ESCAPE,
['u'] = JSON_ESCAPE,
['f'] = JSON_ESCAPE,
['l'] = IN_ESCAPE_L,
['I'] = IN_ESCAPE_I,
Aha, that 'd' should be '[du]' in all of the comments.
> @@ -411,10 +408,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 } }",
Ouch; you are reverting commit 1792d7d0. We tried hard to make
json-lexer.c understand lots of different 64-bit format spellings, but
we KNOW that we don't support MacOS (see commit 29a6731). We either
need to beef up json-lexer.c to understand %qd, or get rid of JSON
psuedo-strings, if we expect this to work; otherwise, you should use
%lld and long long instead of PRId64 and uint64_t.
Overall, I like the patch, but we need to fix the problems for the next
round of this series.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 7/9] tests: Clean up wait for event, (continued)