[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 03/12] qtest: Document calling conventions
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 03/12] qtest: Document calling conventions |
Date: |
Fri, 28 Jul 2017 14:00:17 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 07/28/2017 01:32 PM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> We have two flavors of vararg usage in qtest; make it clear that
>> qmp() has different semantics than hmp(), and let the compiler
>> enforce that hmp() is used correctly. However, qmp() (and friends)
>> only accept a subset of printf flags look-alikes (namely, those
>> that our JSON parser understands), and what is worse, qmp("true")
>> (the JSON keyword 'true') is different from qmp("%s", "true")
>> (the JSON string '"true"'), and we have some intermediate cleanup
>> patches to do before we can mark those as printf-like.
>
> It's not "worse", it's just different :)
>
> Suggest:
>
> We have two flavors of vararg usage in qtest: qtest_hmp() etc. work
> like sprintf(), and qtest_qmp() etc. work like qobject_from_jsonf().
> Spell that out in the comments.
>
> Also add GCC_FMT_ATTR() to qtest_hmp() etc. so that the compiler can
> flag incorrect use.
>
> We have some cleanup work to do before we can do the same for
> qtest_qmp() etc. This will get us the same better-than-nothing
> checking we already have for qobject_from_jsonf(): common incorrect
> uses of supported conversion specifications will be flagged
> (e.g. passing a double for %d), but use of unsupported ones won't.
"Mikey likes it" (no idea if that pop culture reference from my
childhood has broader range than the US)
>> @@ -134,19 +140,19 @@ QDict *qtest_qmp_eventwait_ref(QTestState *s, const
>> char *event);
>> /**
>> * qtest_hmp:
>> * @s: #QTestState instance to operate on.
>> - * @fmt...: HMP command to send to QEMU
>> + * @fmt...: HMP command to send to QEMU, formats arguments like vsprintf().
>
> Like sprintf().
Hmm, you asked me to use vsprintf on the last one. Oh, I finally see -
you're trying to get me to match: vsprintf if it is 'va_list', sprintf
if it is '...'. Yeah, that makes sense.
> With the comment fixed, and preferably with an improved commit message:
> Reviewed-by: Markus Armbruster <address@hidden>
Thanks for the reviews and suggestions.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 05/12] tests: Clean up string interpolation into QMP input (simple cases), Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 06/12] tests/libqos/usb: Clean up string interpolation into QMP input, Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 04/12] tests: Pass literal format strings directly to qmp_FOO(), Eric Blake, 2017/07/25
[Qemu-devel] [PATCH v3 07/12] qtest: Add a new helper qmp_cmd() and friends, Eric Blake, 2017/07/25