Re: [XEN PATCH v3] xen: introduce function type bug_fn_t.

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 17/11/23 11:12, Julien Grall wrote:
> Hi Federico,
>
> On 17/11/2023 08:28, Federico Serafini wrote:
> > Introduce function type bug_fn_t. This improves readability and helps
> > to validate that the function passed to run_in_exception_handle() has
> > the expected prototype.
> Hmmm... I read the second part as you will validate the type in 
> run_in_exception_handle(). But I can't find such change. How about:
> "and could be used to help validating that ..."
>
> No need to send a new revision for that. I can do it on commit.
>
> > Use the newly-intoduced type to address a violation of MISRA
> > C:2012 Rule 8.2.
> >
> > Suggested-by: Julien Grall <julien@xxxxxxx>
> > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> Acked-by: Julien Grall <jgrall@xxxxxxxxxx>
>
> > ---
> >   xen/arch/arm/traps.c  | 2 +-
> >   xen/include/xen/bug.h | 5 +++--
> >   2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index ce89f16404..8492e2b7bb 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -1236,7 +1236,7 @@ int do_bug_frame(const struct cpu_user_regs 
> > *regs, vaddr_t pc)
> >       if ( id == BUGFRAME_run_fn )
> >       {
> > -        void (*fn)(const struct cpu_user_regs *) = (void 
> > *)regs->BUG_FN_REG;
> > +        bug_fn_t *fn = (void *)regs->BUG_FN_REG;
> >           fn(regs);
> >           return 0;
> > diff --git a/xen/include/xen/bug.h b/xen/include/xen/bug.h
> > index e8a4eea71a..cb5138410e 100644
> > --- a/xen/include/xen/bug.h
> > +++ b/xen/include/xen/bug.h
> > @@ -99,6 +99,9 @@ struct bug_frame {
> >   #endif
> > +struct cpu_user_regs;
> > +typedef void bug_fn_t(const struct cpu_user_regs *regs);
> > +
> If your aim is to validate the type in run_in_exception_handler(), then 
> this is defined too late. You will need to define it before "asm/bug.h" 
> is included so arch-specific implementations of run_in_exception_handler 
> can use it.
> Note that for Arm we are using a macro, but others may use a static inline.
Thanks for the information!

--
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)