Markus Armbruster <arm...@redhat.com> writes:
> Alex Bennée <alex.ben...@linaro.org> writes: > >> Markus Armbruster <arm...@redhat.com> writes: >> >>> Back in 2016, we discussed[1] rules for headers, and these were >>> generally liked: >>> >>> 1. Have a carefully curated header that's included everywhere first. We >>> got that already thanks to Peter: osdep.h. >>> >>> 2. Headers should normally include everything they need beyond osdep.h. >>> If exceptions are needed for some reason, they must be documented in >>> the header. If all that's needed from a header is typedefs, put >>> those into qemu/typedefs.h instead of including the header. >>> >>> 3. Cyclic inclusion is forbidden. >>> >>> This patch gets include/ closer to obeying 2. >>> >>> It's actually extracted from my "[RFC] Baby steps towards saner >>> headers" series[2], which demonstrates a possible path towards >>> checking 2 automatically. It passes the RFC test there. >>> >>> [1] Message-ID: <87h9g8j57d....@blackfin.pond.sub.org> >>> https://lists.nongnu.org/archive/html/qemu-devel/2016-03/msg03345.html >>> [2] Message-Id: <20190711122827.18970-1-arm...@redhat.com> >>> https://lists.nongnu.org/archive/html/qemu-devel/2019-07/msg02715.html >>> >>> Signed-off-by: Markus Armbruster <arm...@redhat.com> >>> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> >>> --- >> <snip> >>> include/exec/cputlb.h | 2 ++ >>> include/exec/exec-all.h | 1 + >>> include/exec/ioport.h | 2 ++ >>> include/exec/memory-internal.h | 2 ++ >>> include/exec/ram_addr.h | 1 + >>> include/exec/softmmu-semi.h | 2 ++ >>> include/exec/tb-hash.h | 2 ++ >>> include/exec/user/thunk.h | 1 + >>> include/fpu/softfloat-macros.h | 2 ++ >> <snip> >>> >>> /* >>> * bdrv_write_threshold_set: >>> diff --git a/include/disas/disas.h b/include/disas/disas.h >>> index 15da511f49..ba47e9197c 100644 >>> --- a/include/disas/disas.h >>> +++ b/include/disas/disas.h >>> @@ -1,6 +1,7 @@ >>> #ifndef QEMU_DISAS_H >>> #define QEMU_DISAS_H >>> >>> +#include "exec/hwaddr.h" >>> >>> #ifdef NEED_CPU_H >>> #include "cpu.h" >>> diff --git a/include/exec/cputlb.h b/include/exec/cputlb.h >>> index 5373188be3..23abd71579 100644 >>> --- a/include/exec/cputlb.h >>> +++ b/include/exec/cputlb.h >>> @@ -19,6 +19,8 @@ >>> #ifndef CPUTLB_H >>> #define CPUTLB_H >>> >>> +#include "exec/cpu-common.h" >>> + >>> #if !defined(CONFIG_USER_ONLY) >>> /* cputlb.c */ >>> void tlb_protect_code(ram_addr_t ram_addr); >>> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h >>> index 16034ee651..135aeaab0d 100644 >>> --- a/include/exec/exec-all.h >>> +++ b/include/exec/exec-all.h >>> @@ -20,6 +20,7 @@ >>> #ifndef EXEC_ALL_H >>> #define EXEC_ALL_H >>> >>> +#include "cpu.h" >>> #include "exec/tb-context.h" >>> #include "sysemu/cpus.h" >>> >>> diff --git a/include/exec/ioport.h b/include/exec/ioport.h >>> index a298b89ce1..97feb296d2 100644 >>> --- a/include/exec/ioport.h >>> +++ b/include/exec/ioport.h >>> @@ -24,6 +24,8 @@ >>> #ifndef IOPORT_H >>> #define IOPORT_H >>> >>> +#include "exec/memory.h" >>> + >>> #define MAX_IOPORTS (64 * 1024) >>> #define IOPORTS_MASK (MAX_IOPORTS - 1) >>> >>> diff --git a/include/exec/memory-internal.h b/include/exec/memory-internal.h >>> index d1a9dd1ec8..ef4fb92371 100644 >>> --- a/include/exec/memory-internal.h >>> +++ b/include/exec/memory-internal.h >>> @@ -20,6 +20,8 @@ >>> #ifndef MEMORY_INTERNAL_H >>> #define MEMORY_INTERNAL_H >>> >>> +#include "cpu.h" >>> + >>> #ifndef CONFIG_USER_ONLY >>> static inline AddressSpaceDispatch *flatview_to_dispatch(FlatView *fv) >>> { >>> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h >>> index b7b2e60ff6..a327a80cfe 100644 >>> --- a/include/exec/ram_addr.h >>> +++ b/include/exec/ram_addr.h >>> @@ -20,6 +20,7 @@ >>> #define RAM_ADDR_H >>> >>> #ifndef CONFIG_USER_ONLY >>> +#include "cpu.h" >>> #include "hw/xen/xen.h" >>> #include "sysemu/tcg.h" >>> #include "exec/ramlist.h" >>> diff --git a/include/exec/softmmu-semi.h b/include/exec/softmmu-semi.h >>> index 970837992e..fbcae88f4b 100644 >>> --- a/include/exec/softmmu-semi.h >>> +++ b/include/exec/softmmu-semi.h >>> @@ -10,6 +10,8 @@ >>> #ifndef SOFTMMU_SEMI_H >>> #define SOFTMMU_SEMI_H >>> >>> +#include "cpu.h" >>> + >>> static inline uint64_t softmmu_tget64(CPUArchState *env, target_ulong addr) >>> { >>> uint64_t val; >>> diff --git a/include/exec/tb-hash.h b/include/exec/tb-hash.h >>> index 4f3a37d927..805235d321 100644 >>> --- a/include/exec/tb-hash.h >>> +++ b/include/exec/tb-hash.h >>> @@ -20,6 +20,8 @@ >>> #ifndef EXEC_TB_HASH_H >>> #define EXEC_TB_HASH_H >>> >>> +#include "exec/cpu-defs.h" >>> +#include "exec/exec-all.h" >>> #include "qemu/xxhash.h" >>> >>> #ifdef CONFIG_SOFTMMU >>> diff --git a/include/exec/user/thunk.h b/include/exec/user/thunk.h >>> index 8d3af5a3be..d05a8a4dab 100644 >>> --- a/include/exec/user/thunk.h >>> +++ b/include/exec/user/thunk.h >>> @@ -20,6 +20,7 @@ >>> #define THUNK_H >>> >>> #include "cpu.h" >>> +#include "exec/user/abitypes.h" >>> >>> /* types enums definitions */ >> >> These all seem OK. >> >>> >>> diff --git a/include/fpu/softfloat-macros.h b/include/fpu/softfloat-macros.h >>> index c55aa6d174..be83a833ec 100644 >>> --- a/include/fpu/softfloat-macros.h >>> +++ b/include/fpu/softfloat-macros.h >>> @@ -82,6 +82,8 @@ this code that are retained. >>> #ifndef FPU_SOFTFLOAT_MACROS_H >>> #define FPU_SOFTFLOAT_MACROS_H >>> >>> +#include "fpu/softfloat.h" >>> + >> >> What does softfloat-macros actually need from the core softfloat API? >> These are lower level functions used by softfloat itself (and m68k for >> it's own bit fiddling). > > I extracted this patch out of "[PATCH RFC v5 0/3] Baby steps towards > saner headers". PATCH 1/3 creates make target "check-source", which is > what I used to find headers that aren't self-contained. In this case: > > CC cris-softmmu/tests/headers-tgt/include/fpu/softfloat.o > In file included from tests/headers-tgt/include/fpu/softfloat-macros.c:21: > /work/armbru/qemu/include/fpu/softfloat-macros.h: In function > ‘estimateDiv128To64’: > /work/armbru/qemu/include/fpu/softfloat-macros.h:623:27: error: implicit > declaration of function ‘LIT64’ [-Werror=implicit-function-declaration] > 623 | if ( b <= a0 ) return LIT64( 0xFFFFFFFFFFFFFFFF ); The LIT64 definition should be moved to softfloat-types.h which is already included by softfloat.h unless we already have a QEMU expansion we should be using. The softfloat-macros.h can include softfloat-types.h as well and we should only include the full softfloat.h if they need it. Do you want me to spin up a patch? > | ^~~~~ > /work/armbru/qemu/include/fpu/softfloat-macros.h:623:27: error: nested extern > declaration of ‘LIT64’ [-Werror=nested-externs] > /work/armbru/qemu/include/fpu/softfloat-macros.h: At top level: > /work/armbru/qemu/include/fpu/softfloat-macros.h:761:15: error: unknown type > name ‘flag’ > 761 | static inline flag eq128( uint64_t a0, uint64_t a1, uint64_t b0, > uint64_t b1 ) > | ^~~~ > /work/armbru/qemu/include/fpu/softfloat-macros.h:774:15: error: unknown type > name ‘flag’ > 774 | static inline flag le128( uint64_t a0, uint64_t a1, uint64_t b0, > uint64_t b1 ) > | ^~~~ > /work/armbru/qemu/include/fpu/softfloat-macros.h:787:15: error: unknown type > name ‘flag’ > 787 | static inline flag lt128( uint64_t a0, uint64_t a1, uint64_t b0, > uint64_t b1 ) > | ^~~~ > /work/armbru/qemu/include/fpu/softfloat-macros.h:800:15: error: unknown type > name ‘flag’ > 800 | static inline flag ne128( uint64_t a0, uint64_t a1, uint64_t b0, > uint64_t b1 ) > | ^~~~ > cc1: all warnings being treated as errors > make[1]: *** [/work/armbru/qemu/rules.mak:69: > tests/headers-tgt/include/fpu/softfloat-macros.o] Error 1 -- Alex Bennée
