From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=44200 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OWu8P-0001CD-0b for qemu-devel@nongnu.org; Thu, 08 Jul 2010 12:39:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OWu8N-0004i8-RR for qemu-devel@nongnu.org; Thu, 08 Jul 2010 12:39:36 -0400 Received: from mail-fx0-f45.google.com ([209.85.161.45]:44272) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OWu8N-0004hx-KD for qemu-devel@nongnu.org; Thu, 08 Jul 2010 12:39:35 -0400 Received: by fxm5 with SMTP id 5so578420fxm.4 for ; Thu, 08 Jul 2010 09:39:33 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4C35FF42.8030809@redhat.com> Date: Thu, 08 Jul 2010 18:39:30 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <5692464.1280361277887491249.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <1502374723.1280691277888172604.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> <4C2DB681.7060402@redhat.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 4/4] require #define NEED_GLOBAL_ENV for files that need the global register variable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Paul Brook , qemu-devel@nongnu.org On 07/03/2010 09:23 AM, Blue Swirl wrote: > On Fri, Jul 2, 2010 at 9:50 AM, Paolo Bonzini 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