Re: [Qemu-devel] [PATCH v6 27/29] libqtest: Make qtest_init() accept format string

[Eric Blake <eblake@redhat.com>]

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

On 09/05/2017 07:46 AM, Thomas Huth wrote:
> On 01.09.2017 20:03, Eric Blake wrote:
>> Several callers were formatting a string into a temporary
>> variable before calling qtest_init(); factor that into the
>> common code.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---

>> -QTestState *qtest_init(const char *extra_args);
>> +QTestState *qtest_init(const char *extra_args, ...) GCC_FMT_ATTR(1, 2);
> 
> Maybe the parameter should be called "fmt" or "format" now instead?
> (similar to printf() and friends)

Sure.

> 
>>  /**
>>   * qtest_init_without_qmp_handshake:
>> @@ -518,7 +519,7 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data);
>>   */
>>  static inline QTestState *qtest_start(const char *args)
>>  {
>> -    global_qtest = qtest_init(args);
>> +    global_qtest = qtest_init("%s", args);
> 
> That's a little bit sad that there are some spots which need an explicit
> "%s" now ... maybe it would be nicer to add a new function called
> qtest_initf() instead and keep the old one (without the "f") as it is?

Indeed, and looking back over my series, I see we could also benefit
from a va_list form.

Also, since you already questioned me on the churn from qtest_start() =>
qtest_init() => qtest_start(), I can make less churn in the next spin by
converging on qtest_start() from the get-go.

So I will go with qtest_start() (plain string), qtest_startf()
(formatted like sprintf), and qtest_vstartf() (formatted like vsprintf).

-- 
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 --]