On 23/11/2020 16.59, Cornelia Huck wrote: > On Mon, 23 Nov 2020 11:47:25 +0100 > Markus Armbruster <arm...@redhat.com> wrote: > >> Thomas Huth <th...@redhat.com> writes: >> >>> On 18/11/2020 15.30, Peter Maydell wrote: >>>> On Wed, 18 Nov 2020 at 14:24, Markus Armbruster <arm...@redhat.com> wrote: >>>> >>>>> >>>>> Philippe Mathieu-Daudé <phi...@redhat.com> writes: >>>>> >>>>>> On 11/18/20 10:03 AM, Thomas Huth wrote: >>>>>>> Both headers, sysbus.h and module.h, are not required to compile this >>>>>>> file. >>>>> >>>>> module.h is: it defines type_init(). >>>> >>>>>>> #include "qemu/timer.h" >>>>>>> #include "hw/watchdog/wdt_diag288.h" >>>>>>> #include "migration/vmstate.h" >>>>>>> #include "qemu/log.h" >>>>>>> -#include "qemu/module.h" >>>>>> >>>>>> Cc'ing Markus because of: >>>> >>>>>> Include qemu/module.h where needed, drop it from qemu-common.h >>>>> >>>>> If it still compiles and links, it must get it via some other header. >>>> >>>> Yes: wdt_diag288.c -> include/hw/watchdog/wdt_diag288.h -> >>>> include/qom/object.h -> include/qemu/module.h >>> >>> So what's now our expectation here? Should every file that uses type_init() >>> also include module.h ? That's IMHO not very intuitive... >>> Or are we fine that type_init() is provided by qom/object.h which needs to >>> be pulled in by every device sooner or later anyway? >> >> I think it's okay to rely on indirect inclusion. > > So, what's the final verdict? Maybe just tweak the description? > > "Neither sysbus.h nor module.h are required to compile this file. > diag288 is not a sysbus device, and module.h (for type_init) is > included eventually through qom/object.h."
Yes, I think that's the way to go. Could you update the description when picking up the patch, or shall I send a v2? Thomas
