Eric Blake <ebl...@redhat.com> writes:
> On 07/20/2017 03:37 PM, Eric Blake wrote:
>>>> + * @fmt...: QMP message to send to qemu; only recognizes formats
>>>> + * understood by json-lexer.c
>>>> *
>>>> * Sends a QMP message to QEMU and consumes the response.
>>>> */
>>>
>>> These formats are chosen so that gcc can help us check them. Please add
>>> GCC_FMT_ATTR(). Precedence: qobject_from_jsonf().
>>
>> Will do. (This patch was originally part of my larger series trying to
>> get rid of qobject_from_jsonf() - I may still succeed at that, but
>> separating the easy stuff from the controversial means that the easy
>> stuff needs tweaking).
>
> Bleargh. It's not that simple.
>
> With printf-style, hmp("literal") and hmp("%s", "literal") produce the
> same results.
>
> But with json-lexer style, %s MODIFIES its input.
Your assertion "MODIFIES its input" confused me briefly, because I read
it as "side effect on the argument string". That would be bonkers.
What you mean is it doesn't emit the argument string unmodified.
> The original qmp("{'execute':\"foo\"}") sends a JSON object
> {'execute':"foo"}
> but counterpart qmp("%s", "{'execute':'foo'}") sends a JSON string with
> added \ escaping:
> "{'execute':\"foo\"}"
>
> So adding the format immediately causes lots of warnings, such as:
>
> CC tests/vhost-user-test.o
> tests/vhost-user-test.c: In function ‘test_migrate’:
> tests/vhost-user-test.c:668:5: error: format not a string literal and no
> format arguments [-Werror=format-security]
> rsp = qmp(cmd);
> ^~~
cmd = g_strdup_printf("{ 'execute': 'migrate',"
"'arguments': { 'uri': '%s' } }",
uri);
rsp = qmp(cmd);
g_free(cmd);
g_assert(qdict_haskey(rsp, "return"));
QDECREF(rsp);
For this to work, @uri must not contain funny characters.
Shouldn't we simply use the built-in quoting here?
rsp = qmp("{ 'execute': 'migrate', 'arguments': { 'uri': %s } }", uri);
g_assert(qdict_haskey(rsp, "return"));
QDECREF(rsp);
>
> CC tests/boot-order-test.o
> tests/boot-order-test.c: In function ‘test_a_boot_order’:
> tests/boot-order-test.c:46:26: error: zero-length gnu_printf format
> string [-Werror=format-zero-length]
> qmp_discard_response(""); /* HACK: wait for event */
> ^~
Should use qmp_eventwait(). Doesn't because it predates it.
> but we CAN'T rewrite those to qmp("%s", command).
Got more troublemakers?
> Now you can sort-of understand why my larger series wanted to get rid of
> qobject_from_jsonf(), since our use of GCC_FMT_ATTR() there is a lie?
I don't think it's a lie. qobject_from_jsonf() satisfies the attribute
format(printf, ...) contract: it fetches varargs exactly like printf().
What it does with them is of no concern to this attribute.
