| Message ID | 3942021ff51b117ab2b50aecd6e75353cd73ab20.1700158707.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v3] xen: introduce function type bug_fn_t. | expand |
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@xen.org> > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> Acked-by: Julien Grall <jgrall@amazon.com> > --- > 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. Cheers,
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@xen.org> >> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> > > Acked-by: Julien Grall <jgrall@amazon.com> > >> --- >> 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!
On Fri, 17 Nov 2023, 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. I committed it together with the old patches I was tracking
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); + #ifndef run_in_exception_handler /* @@ -132,8 +135,6 @@ struct bug_frame { #ifdef CONFIG_GENERIC_BUG_FRAME -struct cpu_user_regs; - /* * Returns a negative value in case of an error otherwise * BUGFRAME_{run_fn, warn, bug, assert}
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. Use the newly-intoduced type to address a violation of MISRA C:2012 Rule 8.2. Suggested-by: Julien Grall <julien@xen.org> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- xen/arch/arm/traps.c | 2 +- xen/include/xen/bug.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)