qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Blue Swirl <blauwirbel@gmail.com>
Cc: Amit Shah <amit.shah@redhat.com>,
	Isaku Yamahata <yamahata@valinux.co.jp>,
	qemu-devel@nongnu.org
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	[thread overview]
Message-ID: <4C2857A5.20206@redhat.com> (raw)
In-Reply-To: <AANLkTikFdQFCcaMfc_gTGpp-gg1njv0khAgh5LEeLVKZ@mail.gmail.com>

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	other threads:[~2010-06-28  8:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-25 12:52 [Qemu-devel] [PATCH 0/7] poison TARGET_xxx for compile once object and header file cleanups Paolo Bonzini
2010-06-25 12:52 ` [Qemu-devel] [PATCH 1/7] rtc: Remove TARGET_I386 from qemu-config.c, enables driftfix Paolo Bonzini
2010-06-25 12:52 ` [Qemu-devel] [PATCH 2/7] include qemu-common.h when needed by the next patches Paolo Bonzini
2010-06-25 12:52 ` [Qemu-devel] [PATCH 3/7] include stdio.h freely, remove dyngen-exec.h hacks Paolo Bonzini
2010-06-25 12:52 ` [Qemu-devel] [PATCH 4/7] provide opaque CPUState to files that are compiled once Paolo Bonzini
2010-06-27 19:17   ` Blue Swirl
2010-06-28  8:04     ` Paolo Bonzini [this message]
2010-06-28 14:21       ` Blue Swirl
2010-06-25 12:52 ` [Qemu-devel] [PATCH 5/7] add qdev property type "cpu" Paolo Bonzini
2010-06-26  7:46   ` Markus Armbruster
2010-06-27 12:48     ` [Qemu-devel] " Paolo Bonzini
2010-06-29 11:42       ` Markus Armbruster
2010-06-29 13:52         ` Paolo Bonzini
2010-06-27 18:45   ` [Qemu-devel] " Blue Swirl
2010-06-25 12:52 ` [Qemu-devel] [PATCH 6/7] replace void* uses with opaque CPUState* Paolo Bonzini
2010-06-26  7:49   ` Markus Armbruster
2010-06-25 12:52 ` [Qemu-devel] [PATCH 7/7] poison TARGET_xxx for compile once object Paolo Bonzini
2010-06-25 22:47 ` [Qemu-devel] [PATCH 0/7] poison TARGET_xxx for compile once object and header file cleanups Richard Henderson
2010-06-27 12:39   ` [Qemu-devel] " Paolo Bonzini
2010-06-27 19:32 ` [Qemu-devel] " Blue Swirl
2010-06-28  8:20   ` Paolo Bonzini
2010-06-28 15:11     ` Blue Swirl
2010-06-28 16:21       ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C2857A5.20206@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=yamahata@valinux.co.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).