On 09/05/2017 06:03 AM, Thomas Huth wrote: > 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. >>>
>>> +++ 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? Yes, older gcc fails to compile (my off-hand recollection is that it was a bug in the older gcc, and not a standards-compliance issue to encounter the same typedef twice, but I could be wrong), ergo the fixup that you later noticed. >> >> 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. Eww. I like headers to be self-contained. Other than stuff we get from osdep.h (which we know is included by EVERY .c file), I prefer that if a header uses another type, then it guarantees that an idempotent inclusion of a header that declares that type is also present in the header file, rather than forcing .c files to know which order to include things in. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature