Re: [Qemu-devel] [PATCH 4/7] provide opaque CPUState to files that are compiled once

[Blue Swirl]

On Mon, Jun 28, 2010 at 8:04 AM, Paolo Bonzini <address@hidden> wrote:
> On 06/27/2010 09:17 PM, Blue Swirl wrote:
>>
>> I'm not comfortable with this part. Accidental use of the global
>> register variable can cause subtle bugs. I'd rather rename 'env' to
>> something more obvious and less likely to collide, like
>> 'global_reg_env' and always poison that. Then we could replace 'env1'
>> etc with just 'env'.
>
> This is not very different from before thanks to the reordering of includes
> done in this patch.
>
> All target-*/exec.h files now start with
>
> #include "config.h"
> #include "dyngen-exec.h"
> #include "cpu.h"
> #include "exec-all.h"
>
> // sometimes a few #defines
>
> register struct CPUAlphaState *env asm(AREG0);
>
> And so they cannot use the global env unless NEED_CPU_H is defined.  If
> anything, it's clearer than before because the structure of the initial
> #includes is the same for all targets.
>
> It's true that a "NEED_GLOBAL_ENV" would provide even better safety, but
> that's something for a separate patch series.  It's particularly easy to do
> after replacing CPU<Target>State with CPUState, so that it can be moved into
> exec-all.h, but this series is already big enough IMO.  Let's do cleanups
> one thing at a time please.

Fine, but then let's not unpoison env with this patch set, please.