On Mon, Jun 28, 2010 at 8:04 AM, Paolo Bonzini <pbonz...@redhat.com> 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.