Re: [Qemu-ppc] [Qemu-devel] [PATCH v6 14/29] libqos: Use explicit QTestState for fw_cfg operations

[Thomas Huth]

On 05.09.2017 12:12, Thomas Huth wrote:
> On 01.09.2017 20:03, Eric Blake wrote:
>> Drop one more client of global_qtest by teaching all fw_cfg test
>> functionality (invoked through alloc-pc) to pass in an explicit
>> QTestState, adjusting all callers.  In particular, fw_cfg-test
>> had to reorder things to create the test state prior to creating
>> the fw_cfg.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>> ---
>>  tests/libqos/fw_cfg.h       | 10 ++++++----
>>  tests/libqos/libqos.h       |  2 +-
>>  tests/libqos/malloc-pc.h    |  4 ++--
>>  tests/libqos/malloc-spapr.h |  2 +-
>>  tests/libqos/malloc.h       |  1 +
>>  tests/boot-order-test.c     |  6 +++---
>>  tests/e1000e-test.c         |  2 +-
>>  tests/fw_cfg-test.c         | 14 ++++++--------
>>  tests/ide-test.c            |  2 +-
>>  tests/libqos/fw_cfg.c       | 14 ++++++++------
>>  tests/libqos/libqos.c       |  2 +-
>>  tests/libqos/malloc-pc.c    |  8 ++++----
>>  tests/libqos/malloc-spapr.c |  4 ++--
>>  tests/vhost-user-test.c     |  2 +-
>>  14 files changed, 38 insertions(+), 35 deletions(-)
>>
>> diff --git a/tests/libqos/fw_cfg.h b/tests/libqos/fw_cfg.h
>> index e8371b2317..396dd4ee1e 100644
>> --- a/tests/libqos/fw_cfg.h
>> +++ b/tests/libqos/fw_cfg.h
>> @@ -15,10 +15,12 @@
>>
>>
>>  typedef struct QFWCFG QFWCFG;
>> +typedef struct QTestState QTestState;
> 
> Not sure, but I slightly remember that typedeffing a struct like this in
> multiple places can cause compiler warnings or errors with certain
> versions of GCC or clang? So a file that includes both, fw_cfg.h and
> libqtest.h will then fail to compile?
> 
> I think it would be better to change the include order in the .c files
> instead, so that libqtest.h is always included before fw_cfg.h.

Ah, well, I just saw that you also sent a fixup patch for this. Anyway,
I'm not a fan of including header files from other header files, so
changing the include order in the .c files sounds like the better
solution to me.

 Thomas