xen/arch/arm/traps.c | 2 +- xen/include/xen/bug.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-)
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(-)
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}
--
2.34.1
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, -- Julien Grall
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
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! -- Federico Serafini, M.Sc. Software Engineer, BUGSENG (http://bugseng.com)
© 2016 - 2024 Red Hat, Inc.