Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Stefan Weil]

Am 15.07.2012 22:25, schrieb Mike Frysinger:
> This file uses the define HOST_LONG_BITS, but doesn't explicitly include
> qemu-common.h for it leading to build warnings for some setups:
> In file included from qemu/target-bfin/cpu.h:17,
>                   from qemu/cputlb.c:21:
> qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
>   cpu-defs.h |    1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/cpu-defs.h b/cpu-defs.h
> index f49e950..0d6018d 100644
> --- a/cpu-defs.h
> +++ b/cpu-defs.h
> @@ -28,6 +28,7 @@
>   #include <inttypes.h>
>   #include <signal.h>
>   #include "osdep.h"
> +#include "qemu-common.h"
>   #include "qemu-queue.h"
>   #include "targphys.h"


No. Of course this works, but I don't think that it is reasonable
to include qemu-common.h in every *.h file. There are already too
many of them.

target-bfin/cpu.h should start like all other cpu.h files with
these include statements:

#include "config.h"
#include "qemu-common.h"

Regards,

Stefan Weil

[Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Mike Frysinger]

This file uses the define HOST_LONG_BITS, but doesn't explicitly include
qemu-common.h for it leading to build warnings for some setups:
In file included from qemu/target-bfin/cpu.h:17,
                 from qemu/cputlb.c:21:
qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 cpu-defs.h |    1 +
 1 file changed, 1 insertion(+)

diff --git a/cpu-defs.h b/cpu-defs.h
index f49e950..0d6018d 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -28,6 +28,7 @@
 #include <inttypes.h>
 #include <signal.h>
 #include "osdep.h"
+#include "qemu-common.h"
 #include "qemu-queue.h"
 #include "targphys.h"
 
-- 
1.7.9.7

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1289 bytes --]

On Sunday 15 July 2012 15:34:33 Stefan Weil wrote:
> Am 15.07.2012 22:25, schrieb Mike Frysinger:
> > This file uses the define HOST_LONG_BITS, but doesn't explicitly include
> > qemu-common.h for it leading to build warnings for some setups:
> > In file included from qemu/target-bfin/cpu.h:17,
> > 
> >                   from qemu/cputlb.c:21:
> > qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > 
> >   cpu-defs.h |    1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/cpu-defs.h b/cpu-defs.h
> > index f49e950..0d6018d 100644
> > --- a/cpu-defs.h
> > +++ b/cpu-defs.h
> > @@ -28,6 +28,7 @@
> > 
> >   #include <inttypes.h>
> >   #include <signal.h>
> >   #include "osdep.h"
> > 
> > +#include "qemu-common.h"
> > 
> >   #include "qemu-queue.h"
> >   #include "targphys.h"
> 
> No. Of course this works, but I don't think that it is reasonable
> to include qemu-common.h in every *.h file. There are already too
> many of them.
> 
> target-bfin/cpu.h should start like all other cpu.h files with
> these include statements:

sorry, but that's fragile junk.  if a header file uses defines from another 
header file, it should be including it.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Stefan Weil]

Am 15.07.2012 23:54, schrieb Mike Frysinger:
> On Sunday 15 July 2012 15:34:33 Stefan Weil wrote:
>> Am 15.07.2012 22:25, schrieb Mike Frysinger:
>>> This file uses the define HOST_LONG_BITS, but doesn't explicitly include
>>> qemu-common.h for it leading to build warnings for some setups:
>>> In file included from qemu/target-bfin/cpu.h:17,
>>>
>>>                    from qemu/cputlb.c:21:
>>> qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined
>>>
>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>> ---
>>>
>>>    cpu-defs.h |    1 +
>>>    1 file changed, 1 insertion(+)
>>>
>>> diff --git a/cpu-defs.h b/cpu-defs.h
>>> index f49e950..0d6018d 100644
>>> --- a/cpu-defs.h
>>> +++ b/cpu-defs.h
>>> @@ -28,6 +28,7 @@
>>>
>>>    #include <inttypes.h>
>>>    #include <signal.h>
>>>    #include "osdep.h"
>>>
>>> +#include "qemu-common.h"
>>>
>>>    #include "qemu-queue.h"
>>>    #include "targphys.h"
>> No. Of course this works, but I don't think that it is reasonable
>> to include qemu-common.h in every *.h file. There are already too
>> many of them.
>>
>> target-bfin/cpu.h should start like all other cpu.h files with
>> these include statements:
> sorry, but that's fragile junk.  if a header file uses defines from another
> header file, it should be including it.
> -mike

There are different ways how things can be done.

Normally, I agree with you that each header file should be complete,
but that's not the QEMU style.

In your special case, it's more important to keep all */cpu.h similar.
qemu/target-bfin/cpu.h is still not part of the official QEMU code,
so it can be fixed before it is committed.

Cheers,

Stefan

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Stefan Weil]

Am 16.07.2012 07:26, schrieb Stefan Weil:
> Am 15.07.2012 23:54, schrieb Mike Frysinger:
>> On Sunday 15 July 2012 15:34:33 Stefan Weil wrote:
>>> Am 15.07.2012 22:25, schrieb Mike Frysinger:
>>>> This file uses the define HOST_LONG_BITS, but doesn't explicitly 
>>>> include
>>>> qemu-common.h for it leading to build warnings for some setups:
>>>> In file included from qemu/target-bfin/cpu.h:17,
>>>>
>>>>                    from qemu/cputlb.c:21:
>>>> qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined
>>>>
>>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>>>> ---
>>>>
>>>>    cpu-defs.h |    1 +
>>>>    1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/cpu-defs.h b/cpu-defs.h
>>>> index f49e950..0d6018d 100644
>>>> --- a/cpu-defs.h
>>>> +++ b/cpu-defs.h
>>>> @@ -28,6 +28,7 @@
>>>>
>>>>    #include <inttypes.h>
>>>>    #include <signal.h>
>>>>    #include "osdep.h"
>>>>
>>>> +#include "qemu-common.h"
>>>>
>>>>    #include "qemu-queue.h"
>>>>    #include "targphys.h"
>>> No. Of course this works, but I don't think that it is reasonable
>>> to include qemu-common.h in every *.h file. There are already too
>>> many of them.
>>>
>>> target-bfin/cpu.h should start like all other cpu.h files with
>>> these include statements:
>> sorry, but that's fragile junk.  if a header file uses defines from 
>> another
>> header file, it should be including it.
>> -mike
>
> There are different ways how things can be done.
>
> Normally, I agree with you that each header file should be complete,
> but that's not the QEMU style.
>
> In your special case, it's more important to keep all */cpu.h similar.
> qemu/target-bfin/cpu.h is still not part of the official QEMU code,
> so it can be fixed before it is committed.
>
> Cheers,
>
> Stefan

IMHO it would be also a clean solution if the */cpu.h no longer include
config.h and qemu-common.h when those files are included in cpu-def.h.

For that solution, your patch could be a starting point, but it needs
more cleanup: include statements which are part of qemu-common.h
need no duplication in cpu-def.h.

- Stefan

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 2067 bytes --]

On Monday 16 July 2012 01:26:50 Stefan Weil wrote:
> Am 15.07.2012 23:54, schrieb Mike Frysinger:
> > On Sunday 15 July 2012 15:34:33 Stefan Weil wrote:
> >> Am 15.07.2012 22:25, schrieb Mike Frysinger:
> >>> This file uses the define HOST_LONG_BITS, but doesn't explicitly
> >>> include qemu-common.h for it leading to build warnings for some
> >>> setups: In file included from qemu/target-bfin/cpu.h:17,
> >>> 
> >>>                    from qemu/cputlb.c:21:
> >>> qemu/cpu-defs.h:83:5: warning: "HOST_LONG_BITS" is not defined
> >>> 
> >>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> >>> ---
> >>> 
> >>>    cpu-defs.h |    1 +
> >>>    1 file changed, 1 insertion(+)
> >>> 
> >>> diff --git a/cpu-defs.h b/cpu-defs.h
> >>> index f49e950..0d6018d 100644
> >>> --- a/cpu-defs.h
> >>> +++ b/cpu-defs.h
> >>> @@ -28,6 +28,7 @@
> >>> 
> >>>    #include <inttypes.h>
> >>>    #include <signal.h>
> >>>    #include "osdep.h"
> >>> 
> >>> +#include "qemu-common.h"
> >>> 
> >>>    #include "qemu-queue.h"
> >>>    #include "targphys.h"
> >> 
> >> No. Of course this works, but I don't think that it is reasonable
> >> to include qemu-common.h in every *.h file. There are already too
> >> many of them.
> >> 
> >> target-bfin/cpu.h should start like all other cpu.h files with
> > 
> >> these include statements:
> > sorry, but that's fragile junk.  if a header file uses defines from
> > another header file, it should be including it.
> > -mike
> 
> There are different ways how things can be done.
> 
> Normally, I agree with you that each header file should be complete,
> but that's not the QEMU style.
> 
> In your special case, it's more important to keep all */cpu.h similar.
> qemu/target-bfin/cpu.h is still not part of the official QEMU code,
> so it can be fixed before it is committed.

a lot of existing files in the top level pull in qemu-common.h.  i don't think 
this is a special case considering it's the first failure i've seen since i 
started the Blackfin port over a year ago.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Qemu-devel] [PATCH] cpu-defs.h: pull in qemu-common.h for HOST_LONG_BITS

[Andreas Färber]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 18.07.2012 14:13, schrieb Mike Frysinger:
> On Monday 16 July 2012 01:26:50 Stefan Weil wrote:
>> Am 15.07.2012 23:54, schrieb Mike Frysinger:
>>> On Sunday 15 July 2012 15:34:33 Stefan Weil wrote:
>>>> Am 15.07.2012 22:25, schrieb Mike Frysinger:
>>>>> This file uses the define HOST_LONG_BITS, but doesn't
>>>>> explicitly include qemu-common.h for it leading to build
>>>>> warnings for some setups: In file included from
>>>>> qemu/target-bfin/cpu.h:17,
>>>>> 
>>>>> from qemu/cputlb.c:21: qemu/cpu-defs.h:83:5: warning:
>>>>> "HOST_LONG_BITS" is not defined
>>>>> 
>>>>> Signed-off-by: Mike Frysinger <vapier@gentoo.org> ---
>>>>> 
>>>>> cpu-defs.h |    1 + 1 file changed, 1 insertion(+)
>>>>> 
>>>>> diff --git a/cpu-defs.h b/cpu-defs.h index f49e950..0d6018d
>>>>> 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -28,6 +28,7 @@
>>>>> 
>>>>> #include <inttypes.h> #include <signal.h> #include
>>>>> "osdep.h"
>>>>> 
>>>>> +#include "qemu-common.h"
>>>>> 
>>>>> #include "qemu-queue.h" #include "targphys.h"
>>>> 
>>>> No. Of course this works, but I don't think that it is
>>>> reasonable to include qemu-common.h in every *.h file. There
>>>> are already too many of them.
>>>> 
>>>> target-bfin/cpu.h should start like all other cpu.h files
>>>> with
>>> 
>>>> these include statements:
>>> sorry, but that's fragile junk.  if a header file uses defines
>>> from another header file, it should be including it. -mike
>> 
>> There are different ways how things can be done.
>> 
>> Normally, I agree with you that each header file should be
>> complete, but that's not the QEMU style.
>> 
>> In your special case, it's more important to keep all */cpu.h
>> similar. qemu/target-bfin/cpu.h is still not part of the official
>> QEMU code, so it can be fixed before it is committed.
> 
> a lot of existing files in the top level pull in qemu-common.h.  i
> don't think this is a special case considering it's the first
> failure i've seen since i started the Blackfin port over a year
> ago.

Long-term the cpu-defs.h header should go away, so moving stuff into
it and dropping it elsewhere seems not the best of ideas.

qemu-common.h includes cpu.h under some circumstances, which will
include cpu-defs.h, so a circular dependency, not a good idea either.

CPU is pretty tricky terrain. I've been working on improving it but
progress is slow because a solution for one thing tends to conflict
with someone else's work...

Andreas

- -- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.18 (GNU/Linux)

iQIcBAEBAgAGBQJQBrbEAAoJEPou0S0+fgE/WdYP/RMYVj96Rig55BaByc6G0T8w
iBFJ9WcALzna9pAtjkj716K9nsEGxF5/s8Z3t7MpCznMdZbeQeKko2pJeB0fbqqg
9Gj7ErpkHBQvo4v4UQ99MX2/cmqfDjAZ8a25GK0KP9MW32uqFK2mUOSt7f9nKyMm
HtJKRhdsTrO0x3zm5i+A3jEyMmbduU0WKB8bwIk6xiwmVRbqRvc/M2RqOyG9WnFf
LRJpNm7yXJlzShcmaNbtl7+DyUN6CX4cGUQ85l7gRrzpRQIJrGjMOJqwhdQotvRc
r02AgzLaQHVn26mTFT+LvUVIOhNMH/+uaDITmMdUyumcytAZAu17EhgWir/qtNNq
rQe82UVSghW6Os6oS/NR+8UOAfpgWGaUra1xxoiJIO+h+OO0smx2yLSicaKR6n1R
isGAx8KaSI/ypCdECZu14U2bnysYUUeGnpXAOqcx/gh+LP2riE2qT5qf22799+U2
lpgb+Vodfq6u4+xThU3aoRtWXMU+5nHnsx++6FZdgjzdesJgYdBuIfG1IH2prOdO
8Q1JVMndDwsRYiXJ9MB/v7e9kEo6JqqZA+V2hHLVmmP9SMuiCUEWzxwfYqKyryuA
eJw2MRHF0Bx68mAmqMfiU+TY6xogpG0sUrLkchoFKuTwpF4HEDBBiN/3ciMSpnak
kWYgVZrUpL7jDaEQsSZl
=x1X7
-----END PGP SIGNATURE-----