[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes
From: |
Cornelia Huck |
Subject: |
Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes |
Date: |
Tue, 24 Nov 2020 12:02:44 +0100 |
On Mon, 23 Nov 2020 19:41:40 +0100
Thomas Huth <thuth@redhat.com> wrote:
> On 23/11/2020 16.59, Cornelia Huck wrote:
> > 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."
>
> 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.
- [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Thomas Huth, 2020/11/18
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Christian Borntraeger, 2020/11/18
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Philippe Mathieu-Daudé, 2020/11/18
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Markus Armbruster, 2020/11/18
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Peter Maydell, 2020/11/18
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Thomas Huth, 2020/11/23
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Markus Armbruster, 2020/11/23
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Cornelia Huck, 2020/11/23
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Thomas Huth, 2020/11/23
- Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes,
Cornelia Huck <=
Re: [PATCH] hw/watchdog/wdt_diag288: Remove unnecessary includes, Cornelia Huck, 2020/11/24