"Michael S. Tsirkin" <m...@redhat.com> writes: > On Fri, Jan 27, 2023 at 10:01:57AM -0500, Michael S. Tsirkin wrote: >> On Fri, Jan 27, 2023 at 02:54:30PM +0000, Peter Maydell wrote: >> > On Thu, 19 Jan 2023 at 14:42, Warner Losh <i...@bsdimp.com> wrote: >> > > >> > > Also, why didn't you move sys/resource.h and other such files >> > > to os-dep.h? I'm struggling to understand the rules around what >> > > is or isn't included where? >> > >> > The rough rule of thumb is that if some OS needs a compatibility >> > fixup or workaround for a system header (eg not every mmap.h >> > defines MAP_ANONYMOUS; on Windows unistd.h has to come before >> > time.h) then we put that header include and the compat workaround >> > into osdep.h. This avoids "only fails on obscure platform" issues >> > where somebody puts a header include into some specific .c file >> > but not the compat workaround, and it works on the Linux host >> > that most people develop and test on and we only find the >> > problem later. >> > >> > There's also no doubt some includes there for historical >> > reasons, and some which really are "everybody needs these" >> > convenience ones. But we should probably not add new >> > includes to osdep.h unless they fall into the "working around >> > system header issues" bucket. >> > >> > thanks >> > -- PMM >> >> >> BTW maybe we should teach checkpatch about that rule: >> if a header is in osdep do not include it directly. > > To be more precise, make checkpatch run clean-includes somehow? > Or just make CI run clean-includes on the tree and verify result > is empty?
scripts/clean-includes isn't quite happy even after my series. Offenders: ebpf/rss.bpf.skeleton.h subprojects/libvduse/libvduse.h subprojects/libvhost-user/libvhost-user-glib.h subprojects/libvhost-user/libvhost-user.h target/hexagon/idef-parser/idef-parser.h target/hexagon/idef-parser/parser-helpers.h tests/fp/platform.h contrib/plugins/cache.c contrib/plugins/drcov.c contrib/plugins/execlog.c contrib/plugins/hotblocks.c contrib/plugins/hotpages.c contrib/plugins/howvec.c contrib/plugins/hwprofile.c contrib/plugins/lockstep.c linux-user/mips64/cpu_loop.c linux-user/mips64/signal.c linux-user/x86_64/cpu_loop.c linux-user/x86_64/signal.c plugins/core.c plugins/loader.c scripts/xen-detect.c subprojects/libvduse/libvduse.c subprojects/libvhost-user/libvhost-user-glib.c subprojects/libvhost-user/libvhost-user.c subprojects/libvhost-user/link-test.c target/hexagon/gen_dectree_import.c target/hexagon/gen_semantics.c target/hexagon/idef-parser/parser-helpers.c target/s390x/gen-features.c tests/migration/s390x/a-b-bios.c tests/plugin/bb.c tests/plugin/empty.c tests/plugin/insn.c tests/plugin/mem.c tests/plugin/syscall.c tests/uefi-test-tools/UefiTestToolsPkg/BiosTablesTest/BiosTablesTest.c tests/unit/test-rcu-simpleq.c tests/unit/test-rcu-slist.c tests/unit/test-rcu-tailq.c tools/ebpf/rss.bpf.c To support automatic checking, we'd have to fix the ones that need need fixing, and add the remainder to the script's XDIRREGEX.