Hi Xenia,
> On 4 Jul 2022, at 08:22, Xenia Ragiadakou <[email protected]> wrote:
>
> The functions show_registers() and show_stack() are referenced only in
> traps.c.
> Change their linkage from external to internal by adding the storage-class
> specifier static to their definitions and by removing show_registers() from
> asm/processor.h header file.
>
> Also, this patch resolves a MISRA C 2012 Rule 8.4 violation warning about the
> function show_stack().
>
> Signed-off-by: Xenia Ragiadakou <[email protected]>
> ---
> I am not 100% sure about this patch.
> I think show_stack() should be declared the same way as show_registers().
> So either both of them will be declared with external linkage or both of them
> will be declared with internal linkage.
I think that those 2 should be declared with external linkage with a comment
explaining why they are. For me those are useful when developing or debugging
and I sometime call those to force dumping the status.
So I would vote to keep the external linkage.
> I decided to declare both of them static because they are referenced only in
> traps.c but I could have, also, add the declaration of show_stack() in
> asm/processor.h header instead. Rule 8.7 is advisory.
As said I would vote for external linkage here but would be nice to have other
developers view on this.
Cheers
Bertrand
>
> xen/arch/arm/include/asm/processor.h | 1 -
> xen/arch/arm/traps.c | 4 ++--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/processor.h
> b/xen/arch/arm/include/asm/processor.h
> index 4188ec6bfb..75c680ae9a 100644
> --- a/xen/arch/arm/include/asm/processor.h
> +++ b/xen/arch/arm/include/asm/processor.h
> @@ -558,7 +558,6 @@ extern register_t __cpu_logical_map[];
> void panic_PAR(uint64_t par);
>
> void show_execution_state(const struct cpu_user_regs *regs);
> -void show_registers(const struct cpu_user_regs *regs);
> //#define dump_execution_state()
> run_in_exception_handler(show_execution_state)
> #define dump_execution_state() WARN()
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 785f2121d1..9398ceeed5 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -931,7 +931,7 @@ static void _show_registers(const struct cpu_user_regs
> *regs,
> printk("\n");
> }
>
> -void show_registers(const struct cpu_user_regs *regs)
> +static void show_registers(const struct cpu_user_regs *regs)
> {
> struct reg_ctxt ctxt;
> ctxt.sctlr_el1 = READ_SYSREG(SCTLR_EL1);
> @@ -1146,7 +1146,7 @@ static void show_trace(const struct cpu_user_regs *regs)
> printk("\n");
> }
>
> -void show_stack(const struct cpu_user_regs *regs)
> +static void show_stack(const struct cpu_user_regs *regs)
> {
> register_t *stack = STACK_BEFORE_EXCEPTION(regs), addr;
> int i;
> --
> 2.34.1
>
