All of lore.kernel.org
 help / color / mirror / Atom feed
From: Xuebing wang <xbing6@gmail.com>
To: Paolo Bonzini <pbonzini@redhat.com>, qemu-devel@nongnu.org
Cc: afaerber@suse.de, stefanha@redhat.com
Subject: Re: [Qemu-devel] [Discussion 02/10] NEED_CPU_H: remove '#include "cpu.h"' from include/qemu-common.h
Date: Tue, 04 Mar 2014 19:54:45 +0800	[thread overview]
Message-ID: <5315BF05.2060500@gmail.com> (raw)
In-Reply-To: <5315A89F.40700@redhat.com>

Hi Paolo, thanks for reviewing.


On 03/04/2014 06:19 PM, Paolo Bonzini wrote:
> Hi,
>
> in general I agree with this patch.  I have a few comments, and I 
> suggest that you split it in multiple patches so that it's easier to 
> get it in when each part is ready.

I spent some time trying to reduce it. I may be wrong, but this is the 
smallest patch I can get to get all targets (linuxuser vs softmmu, kvm 
enable/disable) build.

>
>> diff --git a/hw/dma/soc_dma.c b/hw/dma/soc_dma.c
>> index c06aabb..950f3ec 100644
>> --- a/hw/dma/soc_dma.c
>> +++ b/hw/dma/soc_dma.c
>> @@ -21,6 +21,11 @@
>>  #include "qemu/timer.h"
>>  #include "hw/arm/soc_dma.h"
>>
>> +#ifndef NEED_CPU_H
>> +#error target-xxx/cpu.h must be included because target-specific are 
>> required
>> +#endif
>
> Not needed; include/exec/cpu-defs.h already has a similar #error.

I guess this is my personal preference.

My original idea is to emphasize that this *whole* file is 
"target-specific", as "target-independent" vs "target-specific" is 
important for qemu framework (everything except target-xxx, tcg backend 
and hw/arch*/). Ideally all qemu framework is designed to be 
target-independent, right?

In my docs/api-hierarchy.txt, I am trying my best to illustrate what are 
"target-independent", and what are "target-specific".

cpu-defs.h is NOT included in this file.

The idea is the same for the rest "#ifndef NEED_CPU_H"

     Why use "#ifndef NEED_CPU_H, then #error ...", is because 
"target-xxx/cpu.h"
     is better included for the whole file.

     If "target-xxx/cpu.h" is only needed for part of the file, use
     "#ifdef NEED_CPU_H, then #include "cpu.h"".


Thanks.

  reply	other threads:[~2014-03-04 11:55 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-03-04  2:47 [Qemu-devel] [Discussion 00/10] about API hierarchy Xuebing Wang
2014-03-04  2:47 ` [Qemu-devel] [Discussion 01/10] docs: add docs/api-hierarchy.txt Xuebing Wang
2014-03-04  9:42   ` Stefan Hajnoczi
2014-03-04  9:58     ` Xuebing wang
2014-03-04 11:57       ` Stefan Hajnoczi
2014-03-04  2:47 ` [Qemu-devel] [Discussion 02/10] NEED_CPU_H: remove '#include "cpu.h"' from include/qemu-common.h Xuebing Wang
2014-03-04 10:19   ` Paolo Bonzini
2014-03-04 11:54     ` Xuebing wang [this message]
2014-03-04 12:02     ` Xuebing wang
2014-03-04 12:09       ` Paolo Bonzini
2014-03-04 12:09     ` Xuebing wang
2014-03-04 12:34       ` Peter Maydell
2014-03-04 12:40         ` Xuebing wang
2014-03-04 12:19     ` Xuebing wang
2014-03-04 12:23       ` Paolo Bonzini
2014-03-04 12:26     ` Xuebing wang
2014-03-04 12:29       ` Paolo Bonzini
2014-03-04  2:47 ` [Qemu-devel] [Discussion 03/10] NEED_CPU_H: remove unnecessary use of NEED_CPU_H Xuebing Wang
2014-03-04 10:20   ` Paolo Bonzini
2014-03-04  2:47 ` [Qemu-devel] [Discussion 04/10] memory_mapping: make this architecture-independent Xuebing Wang
2014-03-04 10:22   ` Paolo Bonzini
2014-03-04 11:05   ` Peter Maydell
2014-03-04  2:47 ` [Qemu-devel] [Discussion 05/10] NEED_CPU_H: remove unnecessary inclusion of "cpu.h" in root Xuebing Wang
2014-03-04 10:24   ` Paolo Bonzini
2014-03-04  2:47 ` [Qemu-devel] [Discussion 06/10] memory: move contents in include/exec/address-spaces.h => memory.h Xuebing Wang
2014-03-04 10:26   ` Paolo Bonzini
2014-03-04  2:47 ` [Qemu-devel] [Discussion 07/10] memory: remove file include/exec/address-spaces.h Xuebing Wang
2014-03-04  2:47 ` [Qemu-devel] [Discussion 08/10] exec: move TranslationBlock API from exec-all.h => translate.h Xuebing Wang
2014-03-04 10:27   ` Paolo Bonzini
2014-03-04  2:47 ` [Qemu-devel] [Discussion 09/10] exec: remove the unnecessary include of "exec-all.h" Xuebing Wang
2014-03-04 10:27   ` Paolo Bonzini
2014-03-04 11:11   ` Peter Maydell
2014-03-04 11:16     ` Peter Maydell
2014-03-04  2:47 ` [Qemu-devel] [Discussion 10/10] translate: remove file translate-all.h Xuebing Wang
2014-03-04 10:29   ` Paolo Bonzini
2014-03-04  3:45 ` [Qemu-devel] [Discussion 00/10] about API hierarchy Andreas Färber
2014-03-04  5:37   ` Xuebing wang

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=5315BF05.2060500@gmail.com \
    --to=xbing6@gmail.com \
    --cc=afaerber@suse.de \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.