[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h |
Date: |
Thu, 12 Jan 2023 12:43:23 -0500 |
On Thu, Jan 12, 2023 at 03:58:56PM +0000, Peter Maydell wrote:
> On Thu, 12 Jan 2023 at 15:14, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Thu, Jan 12, 2023 at 08:51:26AM -0500, Michael S. Tsirkin wrote:
> > > On Thu, Jan 12, 2023 at 12:50:05PM +0100, Markus Armbruster wrote:
> > > > docs/devel/style.rst mandates:
> > > >
> > > > The "qemu/osdep.h" header contains preprocessor macros that affect
> > > > the behavior of core system headers like <stdint.h>. It must be
> > > > the first include so that core system headers included by external
> > > > libraries get the preprocessor macros that QEMU depends on.
> > > >
> > > > Do not include "qemu/osdep.h" from header files since the .c file
> > > > will have already included it.
> > > >
> > > > A few violations have crept in. Fix them.
> > > >
> > > > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> > > > Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>
> > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > >
> > > With my awesome grep skillz I found one more:
> > > $ grep -r --include='*.h' qemu/osdep.h
> > > include/block/graph-lock.h:#include "qemu/osdep.h"
> > >
> > > Looks like all C files must include qemu/osdep.h, no?
> >
> > Yes, and IMHO that is/was a mistake, as it means our other header
> > files are not self-contained, which prevents developer tools from
> > reporting useful bugs when you're editting.
>
> The underlying requirement is "osdep.h must be included
> before any system header file". "Always first in every .c file"
> is an easy way to achieve that, and "never in any .h file" is
> then not mandatory but falls out from the fact that any
> such include is pointless and only serves to increase
> the compilation time (and to increase the chances that
> you don't notice that you forgot osdep.h in your .c file).
>
> If there's a better way to do this (e.g. one which meant
> that it was a compile error to put osdep includes in the
> wrong place or to omit them) then that would certainly
> save us periodically having to do this kind of fixup commit.
>
> thanks
> -- PMM
yes I just posted a patch that will catch most (though not all)
such cases. if we switch to -include it will catch all of them
but there seems to be some resistance to this idea.
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, (continued)
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Daniel P . Berrangé, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Peter Maydell, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Daniel P . Berrangé, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Peter Maydell, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Daniel P . Berrangé, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Peter Maydell, 2023/01/12
- Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h,
Michael S. Tsirkin <=
Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Jonathan Cameron, 2023/01/12
Re: [PATCH v3 1/1] include: Don't include qemu/osdep.h, Michael S. Tsirkin, 2023/01/12