[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN PATCH v2 1/7] xen: add declarations for variables where needed
On 16/10/2023 16:50, Jan Beulich wrote: On 09.10.2023 08:54, Nicola Vetrini wrote:Some variables with external linkage used in C code do not have a visible declaration where they are defined. Providing such declaration also resolves violations of MISRA C:2012 Rule 8.4. Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>This is a mix of different approaches to the same underlying issue. I think respectively splitting would have helped.--- a/xen/arch/x86/include/asm/compat.h +++ b/xen/arch/x86/include/asm/compat.h @@ -13,6 +13,7 @@ typedef unsigned long full_ptr_t; struct domain; #ifdef CONFIG_PV32 +extern unsigned long cr4_pv32_mask;Why is this needed? Didn't we say declarations aren't needed when the only consumer is assembly code? (I also wonder how this header is any more "appropriate" - see the changelog entry - than about any other one.) It was pointed out to me [1] that compat.h might be more appropriate than setup.h (probably because the asm code referencing it is under x86_64/compat).Further, while it's true that this variable is used in asm, it's also used in x86/setup.c, hence the need for a declaration. --- a/xen/arch/x86/include/asm/setup.h +++ b/xen/arch/x86/include/asm/setup.h @@ -13,6 +13,7 @@ extern char __2M_rwdata_start[], __2M_rwdata_end[]; extern unsigned long xenheap_initial_phys_start; extern uint64_t boot_tsc_stamp; +extern char cpu0_stack[STACK_SIZE];Same question here. This one is a bit more nuanced (I wouldn't oppose deviating this), but there is indeed one use. --- a/xen/common/symbols.c +++ b/xen/common/symbols.c @@ -21,23 +21,6 @@ #include <xen/guest_access.h> #include <xen/errno.h> -#ifdef SYMBOLS_ORIGIN -extern const unsigned int symbols_offsets[]; -#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n]) -#else -extern const unsigned long symbols_addresses[]; -#define symbols_address(n) symbols_addresses[n] -#endif -extern const unsigned int symbols_num_syms; -extern const u8 symbols_names[]; - -extern const struct symbol_offset symbols_sorted_offsets[]; - -extern const u8 symbols_token_table[]; -extern const u16 symbols_token_index[]; - -extern const unsigned int symbols_markers[]; -/* expand a compressed symbol data into the resulting uncompressed string, given the offset to where the symbol is in the compressed stream */ static unsigned int symbols_expand_symbol(unsigned int off, char *result)--- a/xen/include/xen/symbols.h +++ b/xen/include/xen/symbols.h @@ -33,4 +33,22 @@ struct symbol_offset { uint32_t stream; /* .. in the compressed stream.*/ uint32_t addr; /* .. and in the fixed size address array. */ }; + +#ifdef SYMBOLS_ORIGIN +extern const unsigned int symbols_offsets[]; +#define symbols_address(n) (SYMBOLS_ORIGIN + symbols_offsets[n]) +#else +extern const unsigned long symbols_addresses[]; +#define symbols_address(n) symbols_addresses[n] +#endif +extern const unsigned int symbols_num_syms; +extern const u8 symbols_names[]; + +extern const struct symbol_offset symbols_sorted_offsets[]; + +extern const u8 symbols_token_table[]; +extern const u16 symbols_token_index[]; + +extern const unsigned int symbols_markers[]; + #endif /*_XEN_SYMBOLS_H*/This change is even less clear to me: The producer is assembly code, and the consumer already had appropriate declarations. Why would we want to increase the scope of their visibility? Jan The missing decls are about common/symbols-dummy.c. Xen can choose that this file does not need to conform (to this guideline or any guideline), in which case this change can be dropped. [1] https://lore.kernel.org/xen-devel/ZRqkbeVUZbjizjNv@MacBookPdeRoger/ -- Nicola Vetrini, BSc Software Engineer, BUGSENG srl (https://bugseng.com)
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |