Re: [Qemu-devel] [PATCH 5/5] qtest: Document calling conventions

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2795 bytes --]

On 07/20/2017 05:10 AM, 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. Since qmp() only accepts
>> a subset of printf flags (namely, those that our JSON parser
>> understands), I figured that it is probably not worth adding a
>> format attribute to qmp() at this time.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>  tests/libqtest.h | 23 ++++++++++++++---------
>>  1 file changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index 38bc1e9..762ed13 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -50,7 +50,8 @@ void qtest_quit(QTestState *s);
>>  /**
>>   * qtest_qmp_discard_response:
>>   * @s: #QTestState instance to operate on.
>> - * @fmt...: QMP message to send to qemu
>> + * @fmt...: QMP message to send to qemu; only recognizes formats
>> + * understood by json-lexer.c
>>   *
>>   * Sends a QMP message to QEMU and consumes the response.
>>   */
> 
> These formats are chosen so that gcc can help us check them.  Please add
> GCC_FMT_ATTR().  Precedence: qobject_from_jsonf().

Will do.  (This patch was originally part of my larger series trying to
get rid of qobject_from_jsonf() - I may still succeed at that, but
separating the easy stuff from the controversial means that the easy
stuff needs tweaking).

> 
> Where are the "formats understood by json-lexer.c" documented?

Near the top of qobject/json-lexer.c:

 * Extension for vararg handling in JSON construction:
 *
 * %((l|ll|I64)?d|[ipsf])

Note that %i differs from %d (the former produces true/false, while the
latter produces "42" and friends - but it happens to "work" with gcc's
-Wformat checking, thanks to var-arg type promotion rules).

I could just spell it out directly (in fact, in the above-mentioned
larger series, I still kept qtest_qmp() able to understand %s, but
dropped all the other formats like %ld and %p).

>> + * @fmt...: HMP command to send to QEMU, passed through sprintf()
> 
> Not actually through sprintf(), but I get what you mean.  I like to
> document such things as "Format arguments like vsprintf()."

Works for me.

>> @@ -592,13 +597,13 @@ static inline QDict *qmp_eventwait_ref(const char *event)
>>
>>  /**
>>   * hmp:
>> - * @fmt...: HMP command to send to QEMU
>> + * @fmt...: HMP command to send to QEMU, passed through printf()
> 
> Here, you claim printf().  Typo?

Or inconsistent copy-and-paste.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]