[Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable

[Paolo Bonzini]

On 07/03/2010 09:23 AM, Blue Swirl wrote:
> On Fri, Jul 2, 2010 at 9:50 AM, Paolo Bonzini<address@hidden>  wrote:
> > The *global* env is still unavailable (i.e. no difference WRT poisoning), by
> > virtue of being defined in exec.h which is not available unless -DNEED_CPU_H
> > is defined.
> >
> > So:
> >
> >             | before                          | after
> > ------------+---------------------------------+--------------------------
> > NEED_CPU_H  | env not poisoned, global env    | same
> >             | available iff exec.h included   |
> > ------------+---------------------------------+--------------------------
> > !NEED_CPU_H | env poisoned; CPUState          | env not poisoned;
> >             | not available, so exec.h cannot | exec.h requires cpu.h
> >             | be included                     | so it cannot be included
>
> But why would it be necessary to unpoison 'env' for the lower right
> case? Poisoning protects against accidents (in addition to exec.h
> thing) and there are no downsides.

Because double protection is useless.  Two warnings may be better than 
one, but we're talking about compilation errors and two errors do not 
achieve anything more than one error.  Plus, the policy for code using 
the global `env' is clear and is not going to change, and including 
exec.h outside cpu-exec.c/op_helper.c would be spotted instantly during 
code review.

And the downside, of course, is that you're holding up a patch to 
improve QEMU's type-safety.

If you want to poison env on all files that do not include exec.h that's 
another story.  I think it's a good idea, but it's not what the current 
code does.  So my pending patches would not make anything worse compared 
to the status quo, and it's something that can be done after my existing 
patches go in.

Besides, I'm on vacation and I try to enjoy nature more than hacking for 
these three weeks. :)  So if you want me to rebase/resend, that's half 
hour work and I'll do it gladly, otherwise I'll pass the ball.

Paolo