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

[Blue Swirl]

On Wed, Jun 30, 2010 at 8:56 AM, Paolo Bonzini <address@hidden> wrote:
>>>> Wouldn't it be better to just put this in dyngen-exec.h ?
>>>> AFAICT there's a direct correlation between NEED_GLOBAL_ENV and #include
>>>> "exec.h".
>>>
>>> True, see cover letter in 0/4.  I was told to make each file request
>>> explicitly the global variable though.  So I'd have to leave the #ifdef even
>>> if I moved it into dyngen-exec.h.
>>
>> Well, I only said I'd rename global 'env' to 'global_reg_env', not
>> something about each file requesting it. But NEED_GLOBAL_ENV wasn't so
>> bad idea in my opinion.
>
> It doesn't matter what's the name of the global.

Yes it does. Unfortunately, 'env' is a name which is used for other
purposes than the global register variable pointing to CPUState. Any
of these uses could accidentally refer to global 'env'. Also one bug
was where the function had parameter 'env1', but some code inside
still used 'env'. These kind of bugs can be avoided by simply renaming
'env' to less collision prone name.

>  What matters is
> whether it's defined at all.  For this reason it's bad to bury it
> in dyngen-exec.h which is included only indirectly.  It's better to
> leave it in all */exec.h files as Paul explained---and I agree with
> him.

Fine by me too.

> I also gave reason why unpoisoning env globally is not a problem at
> all.  For target-dependent files, they did not (and do not) poison
> anything, so my first patch series didn't change anything WRT current
> QEMU sources.  exec.h always includes cpu.h, so there's no way exec.h
> can be included by mistake in a target-independent file.  I can make
> exec.h error out if NEED_CPU_H is not defined, but I think it's a
> worthless complication.

I still maintain that 'env' may not be unpoisoned until the name is
less likely to invite accidents.

>
> So, can someone please apply patches 1 to 3 of this series so that
> we can move on?

I think they are nice cleanups regardless of the approach for 'env'.
If nobody has any objections, I'll apply them on Saturday.