On Mon, 23 Nov 2020 19:41:40 +0100 Thomas Huth <th...@redhat.com> wrote:
> 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? No need for a v2, I queued it to s390-next with an updated description.