Re: [PATCH for-7.0 0/4] qemu-common.h include cleanup

[Markus Armbruster]

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.