1. Patchew
  2. Xen
  3. View series

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

Federico Serafini posted 1 patch 2 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/3942021ff51b117ab2b50aecd6e75353cd73ab20.1700158707.git.federico.serafini@bugseng.com
xen/arch/arm/traps.c  | 2 +-
xen/include/xen/bug.h | 5 +++--
2 files changed, 4 insertions(+), 3 deletions(-)
[XEN PATCH v3] xen: introduce function type bug_fn_t.
Posted by Federico Serafini 2 years, 2 months ago 
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
Re: [XEN PATCH v3] xen: introduce function type bug_fn_t.
Posted by Julien Grall 2 years, 2 months ago 
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
Re: [XEN PATCH v3] xen: introduce function type bug_fn_t.
Posted by Stefano Stabellini 2 years, 2 months ago 
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
Re: [XEN PATCH v3] xen: introduce function type bug_fn_t.
Posted by Federico Serafini 2 years, 2 months ago 
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)

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.