qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 4/7] provide opaque CPUState to files that are c


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 4/7] provide opaque CPUState to files that are compiled once
Date: Mon, 28 Jun 2010 10:04:53 +0200
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.10) Gecko/20100621 Fedora/3.0.5-1.fc13 Lightning/1.0b2pre Thunderbird/3.0.5

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]