On 07/28/2017 01:32 PM, Markus Armbruster wrote:
> Eric Blake <eblake@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 <armbru@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