Re: [Qemu-devel] [PATCH 1/5] move qemu_irq typedef out of cpu-common.h

[Anthony Liguori]

Blue Swirl <address@hidden> writes:

> On Mon, Aug 20, 2012 at 11:13 AM, Igor Mammedov <address@hidden> wrote:
>> On Mon, 20 Aug 2012 06:41:04 +0200
>> Stefan Weil <address@hidden> wrote:
>>
>>> Am 20.08.2012 01:39, schrieb Igor Mammedov:
>>> > it's necessary for making CPU child of DEVICE without
>>> > causing circular header deps.
>>> >
>>> > Signed-off-by: Igor Mammedov <address@hidden>
>>> > ---
>>> >   hw/arm-misc.h |    1 +
>>> >   hw/bt.h       |    2 ++
>>> >   hw/devices.h  |    2 ++
>>> >   hw/irq.h      |    2 ++
>>> >   hw/omap.h     |    1 +
>>> >   hw/soc_dma.h  |    1 +
>>> >   hw/xen.h      |    1 +
>>> >   qemu-common.h |    1 -
>>> >   sysemu.h      |    1 +
>>> >   9 files changed, 11 insertions(+), 1 deletions(-)
>>> >
>>> > diff --git a/hw/arm-misc.h b/hw/arm-misc.h
>>> > index bdd8fec..b13aa59 100644
>>> > --- a/hw/arm-misc.h
>>> > +++ b/hw/arm-misc.h
>>> > @@ -12,6 +12,7 @@
>>> >   #define ARM_MISC_H 1
>>> >
>>> >   #include "memory.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   /* The CPU is also modeled as an interrupt controller.  */
>>> >   #define ARM_PIC_CPU_IRQ 0
>>> > diff --git a/hw/bt.h b/hw/bt.h
>>> > index a48b8d4..ebf6a37 100644
>>> > --- a/hw/bt.h
>>> > +++ b/hw/bt.h
>>> > @@ -23,6 +23,8 @@
>>> >    * along with this program; if not, see <http://www.gnu.org/licenses/>.
>>> >    */
>>> >
>>> > +#include "hw/irq.h"
>>> > +
>>> >   /* BD Address */
>>> >   typedef struct {
>>> >       uint8_t b[6];
>>> > diff --git a/hw/devices.h b/hw/devices.h
>>> > index 1a55c1e..c60bcab 100644
>>> > --- a/hw/devices.h
>>> > +++ b/hw/devices.h
>>> > @@ -1,6 +1,8 @@
>>> >   #ifndef QEMU_DEVICES_H
>>> >   #define QEMU_DEVICES_H
>>> >
>>> > +#include "hw/irq.h"
>>> > +
>>> >   /* ??? Not all users of this file can include cpu-common.h.  */
>>> >   struct MemoryRegion;
>>> >
>>> > diff --git a/hw/irq.h b/hw/irq.h
>>> > index 56c55f0..1339a3a 100644
>>> > --- a/hw/irq.h
>>> > +++ b/hw/irq.h
>>> > @@ -3,6 +3,8 @@
>>> >
>>> >   /* Generic IRQ/GPIO pin infrastructure.  */
>>> >
>>> > +typedef struct IRQState *qemu_irq;
>>> > +
>>> >   typedef void (*qemu_irq_handler)(void *opaque, int n, int level);
>>> >
>>> >   void qemu_set_irq(qemu_irq irq, int level);
>>> > diff --git a/hw/omap.h b/hw/omap.h
>>> > index 413851b..8b08462 100644
>>> > --- a/hw/omap.h
>>> > +++ b/hw/omap.h
>>> > @@ -19,6 +19,7 @@
>>> >   #ifndef hw_omap_h
>>> >   #include "memory.h"
>>> >   # define hw_omap_h                "omap.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   # define OMAP_EMIFS_BASE  0x00000000
>>> >   # define OMAP2_Q0_BASE            0x00000000
>>> > diff --git a/hw/soc_dma.h b/hw/soc_dma.h
>>> > index 904b26c..e386ace 100644
>>> > --- a/hw/soc_dma.h
>>> > +++ b/hw/soc_dma.h
>>> > @@ -19,6 +19,7 @@
>>> >    */
>>> >
>>> >   #include "memory.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   struct soc_dma_s;
>>> >   struct soc_dma_ch_s;
>>> > diff --git a/hw/xen.h b/hw/xen.h
>>> > index e5926b7..ff11dfd 100644
>>> > --- a/hw/xen.h
>>> > +++ b/hw/xen.h
>>> > @@ -8,6 +8,7 @@
>>> >    */
>>> >   #include <inttypes.h>
>>> >
>>> > +#include "hw/irq.h"
>>> >   #include "qemu-common.h"
>>> >
>>> >   /* xen-machine.c */
>>> > diff --git a/qemu-common.h b/qemu-common.h
>>> > index e5c2bcd..6677a30 100644
>>> > --- a/qemu-common.h
>>> > +++ b/qemu-common.h
>>> > @@ -273,7 +273,6 @@ typedef struct PCIEPort PCIEPort;
>>> >   typedef struct PCIESlot PCIESlot;
>>> >   typedef struct MSIMessage MSIMessage;
>>> >   typedef struct SerialState SerialState;
>>> > -typedef struct IRQState *qemu_irq;
>>> >   typedef struct PCMCIACardState PCMCIACardState;
>>> >   typedef struct MouseTransformInfo MouseTransformInfo;
>>> >   typedef struct uWireSlave uWireSlave;
>>>
>>> Just move the declaration of qemu_irq to the beginning of qemu-common.h
>>> and leave the rest of files untouched. That also fixes the circular
>>> dependency.
>>>
>>> I already have a patch that does this, so you can integrate it in your
>>> series
>>> instead of this one.
>> No doubt it's more simpler way, but IMHO It's more of a hack than fixing
>> problem.
>> It works for now but doesn't alleviate problem with header nightmare in qemu,
>> where everything is included in qemu-common.h and everything includes it as
>> well.
>>
>> Any way if majority prefer simple move, I'll drop this patch in favor of 
>> yours.
>
> I like Igor's approach more.

Ack.

We should move away from dumping ground includes like qemu-common in
favor of meaningful headers that are explicitly included where needed.

Regards,

Anthony Liguori

>
>>
>>>
>>>
>>> > diff --git a/sysemu.h b/sysemu.h
>>> > index 65552ac..f765821 100644
>>> > --- a/sysemu.h
>>> > +++ b/sysemu.h
>>> > @@ -9,6 +9,7 @@
>>> >   #include "qapi-types.h"
>>> >   #include "notify.h"
>>> >   #include "main-loop.h"
>>> > +#include "hw/irq.h"
>>> >
>>> >   /* vl.c */
>>> >
>>>
>>
>>
>> --
>> Regards,
>>   Igor