Re: [Qemu-devel] [RFC 02/13] qdev: split up header so it can be used in cpu.h

[Andreas Färber]

Am 04.10.2012 21:15, schrieb Eduardo Habkost:
> From: Anthony Liguori <address@hidden>
> 
> Header file dependency is a frickin' nightmare right now.  cpu.h tends to get
> included in our 'include everything' header files but qdev also needs to 
> include
> those headers mainly for qdev-properties since it knows about CharDriverState
> and friends.
> 
> We can solve this for now by splitting out qdev.h along the same lines that we
> previously split the C file.  Then cpu.h just needs to include qdev-core.h
> 
> [ehabkost: re-add DEFINE_PROP_PCI_HOST_DEVADDR, that was removed on the
>  original patch (by mistake, I guess)]
> [ehabkost: kill qdev_prop_set_vlan() declaration]
> 
> Signed-off-by: Anthony Liguori <address@hidden>
> Signed-off-by: Eduardo Habkost <address@hidden>

Generally I am still in favor of doing such a per-topic file split, in
whatever way exactly we proceed with CPU-as-a-device.

If this is supposed to go through my CPU queue then I would prefer to
move the [ehabkost: ...] lines chronologically between the Sobs fwiw.

What I would insist on is mentioning both new file names in the commit
message (qdev-core.h and qdev-properties.h iiuc) since the +++ stats are
ambiguous.

I spotted the following textual discrepancy:

> diff --git a/hw/qdev-core.h b/hw/qdev-core.h
> new file mode 100644
> index 0000000..ca205fc
> --- /dev/null
> +++ b/hw/qdev-core.h
[...]
> +/*
> + * This callback is used to create Open Firmware device path in accordance 
> with
> + * OF spec http://forthworks.com/standards/of1275.pdf. Indicidual bus 
> bindings
> + * can be found here http://playground.sun.com/1275/bindings/.
> + */
[...]
> diff --git a/hw/qdev.h b/hw/qdev.h
> index d699194..365b8d6 100644
> --- a/hw/qdev.h
> +++ b/hw/qdev.h
[...]
> -    /*
> -     * This callback is used to create Open Firmware device path in 
> accordance
> -     * with OF spec http://forthworks.com/standards/of1275.pdf. Individual 
> bus
> -     * bindings can be found at http://playground.sun.com/1275/bindings/.
> -     */
[snip]

"Individual" became "Indicidual" and the comment seems misplaced, which
makes myself ask whether this split was not done safely via copy&paste?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg