On 07/21/2017 01:42 AM, Markus Armbruster wrote: >> 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.
Yes. I'm not sure how I could have worded that better; maybe "does not
do a straight passthrough of its input"
>> 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?
We can - but there are a lot of tests that have to be updated.
>
> 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.
We can - but there are a lot of tests that have to be updated.
>
>> but we CAN'T rewrite those to qmp("%s", command).
>
> Got more troublemakers?
When I worked on getting rid of qobject_from_jsonf(), I was able to
reduce the tests down to JUST using %s, which I then handled locally in
qmp() rather than relying on qobject_from_jsonf(). Going the opposite
direction and more fully relying on qobject_from_jsonf() by fixing
multiple tests that were using older idioms is also an approach (in the
opposite direction I originally took) - but the more work it is, the
less likely that this patch is 2.10 material.
>
>> 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.
I still find it odd that you can't safely turn foo(var) into foo("%s", var).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
