Re: [Qemu-devel] [PATCH 0/7] poison TARGET_xxx for compile once object and header file cleanups

[Richard Henderson <rth@twiddle.net>]

On 06/25/2010 05:52 AM, Paolo Bonzini wrote:
> This is a different way to achieve the same objective as Isamu's patch.
> Basically, his patch becomes the (much simpler) patch 7 of this series,
> and everything else is something I had had lying around for a while. :)
> 
> Patch 1 is simply Amit's patch, included here for convenience as it's
> not been applied yet.
> 
> Patches 2 and 3 remove some dyngen-exec.h hacks at the price of requiring
> qemu-common.h included in more places.  I don't see this as a big price;
> all of these files were already including qemu-common.h indirectly,
> e.g. via cpu-all.h, just not early enough.
> 
> Patches 4 provides a CPUState type, albeit an opaque one, to files that
> are not compiled per-target.  The advantage of this are apparent in 
> patches 5 and 6: opaque pointers that are actually CPUState pointers
> are now type-safe, and it is even possible to define a cpu property type
> for the occasional device that has to be connected to a particular CPU
> (the PC APICs in particular).
> 
> Finally, patch 7 "redoes" Isamu's patch just by moving five lines of
> code into qemu-common.h.
> 
> 
> Amit Shah (1):
>   rtc: Remove TARGET_I386 from qemu-config.c, enables driftfix
> 
> Paolo Bonzini (6):
>   include qemu-common.h when needed by the next patches
>   include stdio.h freely, remove dyngen-exec.h hacks
>   provide opaque CPUState to files that are compiled once
>   add qdev property type "cpu"
>   replace void* uses with opaque CPUState*
>   poison TARGET_xxx for compile once object

Reviewed-by: Richard Henderson <rth@twiddle.net>

I like this cleanup.  Although I would personally prefer an additional
patch that removes the define silliness that patch 4 works around.  In
other words I think there's no point in having CPUARMState et al; we
should use CPUState universally.


r~