From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=60721 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OT9LH-0004cG-V0
	for qemu-devel@nongnu.org; Mon, 28 Jun 2010 04:05:27 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1OT9LF-00033E-Cf
	for qemu-devel@nongnu.org; Mon, 28 Jun 2010 04:05:23 -0400
Received: from mx1.redhat.com ([209.132.183.28]:60240)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <pbonzini@redhat.com>) id 1OT9LF-00032z-6C
	for qemu-devel@nongnu.org; Mon, 28 Jun 2010 04:05:21 -0400
Message-ID: <4C2857A5.20206@redhat.com>
Date: Mon, 28 Jun 2010 10:04:53 +0200
From: Paolo Bonzini <pbonzini@redhat.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH 4/7] provide opaque CPUState to files that
	are compiled once
References: <1277470342-5861-1-git-send-email-pbonzini@redhat.com>
	<1277470342-5861-5-git-send-email-pbonzini@redhat.com>
	<AANLkTikFdQFCcaMfc_gTGpp-gg1njv0khAgh5LEeLVKZ@mail.gmail.com>
In-Reply-To: <AANLkTikFdQFCcaMfc_gTGpp-gg1njv0khAgh5LEeLVKZ@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: Amit Shah <amit.shah@redhat.com>, Isaku Yamahata <yamahata@valinux.co.jp>, qemu-devel@nongnu.org

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.

Paolo