Re: [XEN PATCH v2] xen/arm: traps: address a violation of MISRA C:2012 Rule 8.2

[Federico Serafini <federico.serafini@xxxxxxxxxxx>]

On 08/11/23 17:30, Julien Grall wrote:
> Hi Federico,
>
> On 08/11/2023 16:21, Federico Serafini wrote:
> > On 08/11/23 17:04, Julien Grall wrote:
> > > Hi,
> > >
> > > On 08/11/2023 15:42, Federico Serafini wrote:
> > > > Add missing parameter name "regs" and 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.
> > > > No functional change.
> > > >
> > > > Suggested-by: Julien Grall <julien@xxxxxxx>
> > > > Signed-off-by: Federico Serafini <federico.serafini@xxxxxxxxxxx>
> > > > ---
> > > > Changes in v2:
> > > >    - adjusted tag;
> > > >    - avoided exceeding the 80-character limit.
> > > > ---
> > > >   xen/arch/arm/traps.c | 3 ++-
> > > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > > > index ce89f16404..1264eab087 100644
> > > > --- a/xen/arch/arm/traps.c
> > > > +++ b/xen/arch/arm/traps.c
> > > > @@ -1236,7 +1236,8 @@ 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;
> > > > +        typedef void (*bug_fn_t)(const struct cpu_user_regs *regs);
> > > Thanks for sending a new version. This should be defined in asm/bug.h 
> > > or maybe xen/bug.h as this is generic enough.
> > I see some uses of run_in_exception_handle() in common/bug.c and I guess
> > the type of the actual parameter should be changed to bug_fn_t.
> > Am I missing some other places where such change is needed?
> Just to clarify, right now, I haven't suggested to replace all the 
> open-coding version of the typedef. I am only asking to define the 
> typedef in an header.
OK.

> Note that run_in_exception_handler() is a macro. So you can't really 
> change the type. But you could check that the type of the argument 
> matches bug_fn_t.
I used the wrong terminology but this is what I meant.

--
Federico Serafini, M.Sc.

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