Re: [Qemu-devel] [PATCH v2 5/6] tests/libqtest: Move global_test wrapper function into a separate header

[Eric Blake]

On 9/4/19 1:51 PM, Stefan Hajnoczi wrote:
> On Wed, Sep 04, 2019 at 03:00:46PM +0200, Thomas Huth wrote:
>> diff --git a/tests/libqtest-single.h b/tests/libqtest-single.h
>> new file mode 100644
>> index 0000000000..49259558a5
>> --- /dev/null
>> +++ b/tests/libqtest-single.h

>> +static inline QTestState *qtest_start(const char *args)
>> +{
>> +    global_qtest = qtest_init(args);
> 
> Where are global_qtest and qtest_init() declared?  I would expect
> compilation to fail if a .c file included just "libqtest-single.h".

In patch 5, "libqtest.h" declares global_qtest, then includes
"libqtest-single.h"; no file includes it standalone.  Then in patch 6,
the roles are swapped; "libqtest-single.h" declares global_qtest and
includes "libqtest.h".  But yes, the commit message could do better in
explaining this.

> Missing #include?
> 
> If this header is not supposed to be included by .c files, please
> include at least a comment or use a magic #define + #ifdef to prevent
> inclusion.

The condition is transient for one patch; but calling it out better in
the commit message and/or as a comment (especially since the #include
"libqtest-single.h" is not in the normal #includes up front) would help.

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

signature.asc
Description: OpenPGP digital signature