[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 09/12] tests/libqos/pci: Clean up string inte
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 09/12] tests/libqos/pci: Clean up string interpolation into QMP input |
Date: |
Mon, 31 Jul 2017 13:33:24 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/31/2017 02:29 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> On 07/28/2017 11:35 AM, Eric Blake wrote:
>>>>> + QObject *extra_args = qobject_from_jsonf("{ 'shm': '%s', 'size':
>>>>> '1M' }",
>>>>> + tmpshm);
>>>>
>>
>>> Passing '%s' through qobject_from_jsonf() is generally wrong (it would
>>> produce ''...'' instead of the intended '...').
>>>
>>> Looks like something to fix on the next round.
>>>
>>
>> What's scary is that the testsuite didn't start failing. That's not
>> good; we'll want to figure out why the bug didn't impact the test.
>
> Good idea.
The intended result: the created QDict would include
"shm":"/qtest-11387-3641365368" (or comparable string). The actual
result: the created QDict includes "shm":"%s" - a literal 2-character
string, because the lexer does not do interpolation inside strings. And
apparently, naming your ivshmem shared memory a literal "%s" (rather
than a name beginning with "/") is not a semantic error, so the test passes.
In other words, qobject_from_jsonf() does NOT do % interpolation of
anything embedded inside '', and basically blindly ignores the tmpshm
vararg. It would be neat if we could make qobject_from_jsonf() detect a
mismatch in varargs, even though we don't (can't) require a NULL
sentinel (we're limited to fitting in with -Wformat paradigms already).
So while we can't flag it at compile time, I do think we can flag it at
runtime: if, in qobject_from_jsonf() (but NOT in qobject_from_json()),
we reject any JSON string from the lexer that contains "%" but not "%%",
we will have caught all the places where gcc paired a % sequence with a
vararg parameter even though our lexer did not make the same pairing.
If we WANT a literal % in a string (rare, probably only in the
testsuite), we write it as %% and then qobject_from_jsonf() interpolates
it. Then, since bare % is NOT valid JSON, we don't have to enhance the
lexer to recognize anything new; our changes are limited to
documentation and the vararg parser. It still means we only get a
runtime, rather than a compile-time, detection of misuse of % passed to
the format argument, but it should be an easy enough proof of concept
that such a runtime failure would have been enough to make ivshmem-test
fail and flag the error during 'make check' rather than escaping review.
--
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 v3 07/12] qtest: Add a new helper qmp_cmd() and friends, (continued)
Re: [Qemu-devel] [PATCH v3 07/12] qtest: Add a new helper qmp_cmd() and friends, Markus Armbruster, 2017/07/28
[Qemu-devel] [PATCH v3 09/12] tests/libqos/pci: Clean up string interpolation into QMP input, Eric Blake, 2017/07/25
Re: [Qemu-devel] [PATCH v3 09/12] tests/libqos/pci: Clean up string interpolation into QMP input, Markus Armbruster, 2017/07/28
[Qemu-devel] [PATCH v3 08/12] qtests: convert tests to use qmp_cmd, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 10/12] tests: Clean up wait for event, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 11/12] tests/libqtest: Clean up how we read the QMP greeting, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 12/12] tests/libqtest: Enable compile-time format string checking, Eric Blake, 2017/07/25