| Message ID | 20211129200510.1233037-1-peter.maydell@linaro.org |
|---|---|
| Headers | show |
| Series | qemu-common.h include cleanup | expand |
On Mon, 29 Nov 2021 at 20:05, Peter Maydell <peter.maydell@linaro.org> wrote: > > qemu-common.h has a comment at the top: > > * This file is supposed to be included only by .c files. No header file should > * depend on qemu-common.h, as this would easily lead to circular header > * dependencies. As a side note, that comment was added back in 2012 when qemu-common.h was bigger, included other headers, and did some of the work we currently use osdep.h for. As it stands today qemu-common.h includes no other files so it isn't a source of possible circular dependencies -- it's just a grab-bag of miscellaneous prototypes that in an ideal world would be in more focused individual headers[*]. So there's an argument for deleting this comment... [*] A cleanup that would be nice, and I'm about to send out a patchset that splits out the rtc related functions; but the grab-bag at the bottom of osdep.h is probably higher priority because that header gets pulled in by an order of magnitude more C files. -- PMM
On 11/29/21 9:05 PM, Peter Maydell wrote: > qemu-common.h has a comment at the top: > > * This file is supposed to be included only by .c files. No header file should > * depend on qemu-common.h, as this would easily lead to circular header > * dependencies. > > We still have a few .h files which include it, though. The first 3 > patches in this series fix that: in 3 out of 4 cases we didn't need > the #include at all, and in the 4th case we can instead #include > qemu-common.h from just one .c file. > > Patch 4 is just removing the #include from 8 files in hw/arm which > don't need it at all. (Probably there are other files like this, but > I just did the Arm related ones.) > > Tested by pushing to gitlab for the CI build. > > -- PMM > > Peter Maydell (4): > include/hw/i386: Don't include qemu-common.h in .h files > target/hexagon/cpu.h: don't include qemu-common.h > target/rx/cpu.h: Don't include qemu-common.h > hw/arm: Don't include qemu-common.h unnecessarily Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
Peter Maydell <peter.maydell@linaro.org> writes: > On Mon, 29 Nov 2021 at 20:05, Peter Maydell <peter.maydell@linaro.org> wrote: >> >> qemu-common.h has a comment at the top: >> >> * This file is supposed to be included only by .c files. No header file should >> * depend on qemu-common.h, as this would easily lead to circular header >> * dependencies. > > As a side note, that comment was added back in 2012 when qemu-common.h > was bigger, included other headers, and did some of the work we currently > use osdep.h for. As it stands today qemu-common.h includes no other > files so it isn't a source of possible circular dependencies -- it's > just a grab-bag of miscellaneous prototypes that in an ideal world > would be in more focused individual headers[*]. So there's an argument > for deleting this comment... First, thank you for this cleanup series. The comment is from commit 04509ad939a: qemu-common.h: Comment about usage rules Every time we make a tiny change on a header file, we often find circular header dependency problems. To avoid this nightmare, we need to stop including qemu-common.h from other headers, and we should gradually move the declarations from the catch-all qemu-common.h header to their specific headers. This simply adds a comment documenting the rules about qemu-common.h, hoping that people will see it before including qemu-common.h from other header files, and before adding more declarations to qemu-common.h. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Andreas Färber <afaerber@suse.de> You're right: we've since moved all #include out of qemu-common.h, so the risk of circular header dependency is gone, and the comment's claim "this would easily lead to circular header dependencies" is no longer correct. In my opinion, including qemu-common.h in a header is a bad idea regardless. Headers should include the headers they need to make them self-contained, and no more, because more risks slower compiles, in particular frequent recompiles of pretty much everything. Previously discussed at https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg06291.html Message-ID: <877eac82il.fsf@dusky.pond.sub.org> Nothing in qemu-common.h should be required to make another header self-contained. This is likely the case for other headers as well, which don't carry a comment forbidding inclusion into headers. You could argue that qemu-common.h should not carry one, either. I can accept that. I'm not attached to the comment. I am interested in keeping unwanted #include in headers under control. > [*] A cleanup that would be nice, and I'm about to send out a patchset > that splits out the rtc related functions; but the grab-bag at the > bottom of osdep.h is probably higher priority because that header > gets pulled in by an order of magnitude more C files. By all "normal" ones, in fact.
On 11/29/21 21:05, Peter Maydell wrote: > Peter Maydell (4): > include/hw/i386: Don't include qemu-common.h in .h files > target/hexagon/cpu.h: don't include qemu-common.h > target/rx/cpu.h: Don't include qemu-common.h > hw/arm: Don't include qemu-common.h unnecessarily Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>