qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Isaku Yamahata <yamahata@valinux.co.jp>
Cc: amit.shah@redhat.com, blauwirbel@gmail.com,
	qemu-devel@nongnu.org, rth@twiddle.net
Subject: [Qemu-devel] Re: [PATCH v2] Makefile: poison TARGET_xxx for compile once.
Date: Fri, 25 Jun 2010 10:01:10 +0200	[thread overview]
Message-ID: <4C246246.8030903@redhat.com> (raw)
In-Reply-To: <20100625030240.GJ4498@valinux.co.jp>

On 06/25/2010 05:02 AM, Isaku Yamahata wrote:
> poison TARGET_xxx for compile once object
> to prevent those ifdef from creeping in again.
>
> didn't poison env which is used as function argument as void *env.
> Although it would be possible to sort it out, for now just not poison it.
>
> qemu-malloc.c didn't compile, so I make it non compile-once for now.
> It is linked via block-obj-y in Makefile.obj and common-obj-y in
> Makefile.objs through block-obj-y. So qemu-malloc.o is explicitly
> added to rules.

I'm still skeptical, not about the goal but about the means.

I'm going to push again for my patch to make CPUState opaque for 
non-per-target files.

I haven't heard good reasons against it.  The main objection was that hw 
files would have no reason for accessing CPUState.  But this makes no 
sense if CPUState is opaque, and on the other hand we have now a 
proliferation of void* arguments and fields (e.g. in qemu_cpu_kick). 
Which I am taught is a very bad thing.

If that patch was accepted, we'd just need this to implement your proposal:

diff --git a/cpu-common.h b/cpu-common.h
index f325e60..78f8b12 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -11,10 +11,6 @@
  #include "targphys.h"
  #endif

-#ifndef NEED_CPU_H
-#include "poison.h"
-#endif
-
  #include "bswap.h"
  #include "qemu-queue.h"

diff --git a/qemu-common.h b/qemu-common.h
index 3fb2f0b..3f92d40 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -90,15 +90,12 @@ static inline char *realpath(const char *path, char 
*resolved_path)

  /* FIXME: Remove NEED_CPU_H.  */
  #ifndef NEED_CPU_H
-
  #include <setjmp.h>
  #include "osdep.h"
  #include "bswap.h"
-
+#include "poison.h"
  #else
-
  #include "cpu.h"
-
  #endif /* !defined(NEED_CPU_H) */

  /* bottom halves */


I'll put this together in a complete patch series and post.

Paolo

      reply	other threads:[~2010-06-25 17:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-25  3:02 [Qemu-devel] [PATCH v2] Makefile: poison TARGET_xxx for compile once Isaku Yamahata
2010-06-25  8:01 ` Paolo Bonzini [this message]

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=4C246246.8030903@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=amit.shah@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --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).