| Message ID | 1342383931-11594-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
-----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-----
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"
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(+)