Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes

[Cornelia Huck]

On Mon, 23 Nov 2020 11:47:25 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Thomas Huth <thuth@redhat.com> writes:
> 
> > On 18/11/2020 15.30, Peter Maydell wrote:  
> >> On Wed, 18 Nov 2020 at 14:24, Markus Armbruster <armbru@redhat.com> wrote: 
> >>  
> >>>
> >>> Philippe Mathieu-Daudé <philmd@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."