On 07/28/2017 01:32 PM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> 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 <arm...@redhat.com> 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
