On 07/03/2010 09:23 AM, Blue Swirl wrote:
On Fri, Jul 2, 2010 at 9:50 AM, Paolo Bonzini<pbonz...@redhat.com> 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
