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

[Thomas Huth <thuth@redhat.com>]

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

On 07.09.2017 20:00, Eric Blake wrote:
> 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).

Thanks, that sounds like a better way to me, too!

 Thomas



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