diff mbox series

[XEN,02/10] arm/cpufeature: address violations of MISRA C:2012 Rule 8.2

Message ID 3b28dca993cac9391b997c1744218cf4062907df.1697207038.git.federico.serafini@bugseng.com (mailing list archive)
State New, archived
Headers show
Series arm: address some violations of MISRA C:2012 Rule 8.2 | expand

Commit Message

Federico Serafini Oct. 13, 2023, 3:24 p.m. UTC
Add missing parameter names, no functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Stefano Stabellini Oct. 13, 2023, 11:12 p.m. UTC | #1
On Fri, 13 Oct 2023, Federico Serafini wrote:
> Add missing parameter names, no functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>  xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
> index 8011076b8c..41e97c23dd 100644
> --- a/xen/arch/arm/include/asm/cpufeature.h
> +++ b/xen/arch/arm/include/asm/cpufeature.h
> @@ -127,8 +127,8 @@ static inline void cpus_set_cap(unsigned int num)
>  struct arm_cpu_capabilities {
>      const char *desc;
>      u16 capability;
> -    bool (*matches)(const struct arm_cpu_capabilities *);
> -    int (*enable)(void *); /* Called on every active CPUs */
> +    bool (*matches)(const struct arm_cpu_capabilities *caps);

all the implementations of matches I found in xen/arch/arm/cpuerrata.c
actually call the parameter "entry"


> +    int (*enable)(void *ptr); /* Called on every active CPUs */

this one instead is "data"


>      union {
>          struct {    /* To be used for eratum handling only */
>              u32 midr_model;
> @@ -448,10 +448,10 @@ struct cpuinfo_arm {
>  
>  extern struct cpuinfo_arm system_cpuinfo;
>  
> -extern void identify_cpu(struct cpuinfo_arm *);
> +extern void identify_cpu(struct cpuinfo_arm *c);
>  
>  #ifdef CONFIG_ARM_64
> -extern void update_system_features(const struct cpuinfo_arm *);
> +extern void update_system_features(const struct cpuinfo_arm *new);
>  #else
>  static inline void update_system_features(const struct cpuinfo_arm *cpuinfo)
>  {
> -- 
> 2.34.1
>
Stefano Stabellini Oct. 13, 2023, 11:34 p.m. UTC | #2
On Fri, 13 Oct 2023, Stefano Stabellini wrote:
> On Fri, 13 Oct 2023, Federico Serafini wrote:
> > Add missing parameter names, no functional change.
> > 
> > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> > ---
> >  xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
> > index 8011076b8c..41e97c23dd 100644
> > --- a/xen/arch/arm/include/asm/cpufeature.h
> > +++ b/xen/arch/arm/include/asm/cpufeature.h
> > @@ -127,8 +127,8 @@ static inline void cpus_set_cap(unsigned int num)
> >  struct arm_cpu_capabilities {
> >      const char *desc;
> >      u16 capability;
> > -    bool (*matches)(const struct arm_cpu_capabilities *);
> > -    int (*enable)(void *); /* Called on every active CPUs */
> > +    bool (*matches)(const struct arm_cpu_capabilities *caps);
> 
> all the implementations of matches I found in xen/arch/arm/cpuerrata.c
> actually call the parameter "entry"
> 
> 
> > +    int (*enable)(void *ptr); /* Called on every active CPUs */
> 
> this one instead is "data"

I committed all the other patches in this series to the my for-4.19 branch
Julien Grall Oct. 16, 2023, 8:55 a.m. UTC | #3
Hi,

On 13/10/2023 16:24, Federico Serafini wrote:
> Add missing parameter names, no functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> ---
>   xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
> index 8011076b8c..41e97c23dd 100644
> --- a/xen/arch/arm/include/asm/cpufeature.h
> +++ b/xen/arch/arm/include/asm/cpufeature.h
> @@ -127,8 +127,8 @@ static inline void cpus_set_cap(unsigned int num)
>   struct arm_cpu_capabilities {
>       const char *desc;
>       u16 capability;
> -    bool (*matches)(const struct arm_cpu_capabilities *);
> -    int (*enable)(void *); /* Called on every active CPUs */
> +    bool (*matches)(const struct arm_cpu_capabilities *caps);
> +    int (*enable)(void *ptr); /* Called on every active CPUs */

How did you come up with the name? The void * seems to be named 'data' 
by the declaration and I think we should be consistent, otherwise this 
is defeating the spirit of MISRA (assuming this is not a violation).

Cheers,
Julien Grall Oct. 16, 2023, 8:58 a.m. UTC | #4
On 14/10/2023 00:34, Stefano Stabellini wrote:
> On Fri, 13 Oct 2023, Stefano Stabellini wrote:
>> On Fri, 13 Oct 2023, Federico Serafini wrote:
>>> Add missing parameter names, no functional change.
>>>
>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>> ---
>>>   xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
>>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
>>> index 8011076b8c..41e97c23dd 100644
>>> --- a/xen/arch/arm/include/asm/cpufeature.h
>>> +++ b/xen/arch/arm/include/asm/cpufeature.h
>>> @@ -127,8 +127,8 @@ static inline void cpus_set_cap(unsigned int num)
>>>   struct arm_cpu_capabilities {
>>>       const char *desc;
>>>       u16 capability;
>>> -    bool (*matches)(const struct arm_cpu_capabilities *);
>>> -    int (*enable)(void *); /* Called on every active CPUs */
>>> +    bool (*matches)(const struct arm_cpu_capabilities *caps);
>>
>> all the implementations of matches I found in xen/arch/arm/cpuerrata.c
>> actually call the parameter "entry"
>>
>>
>>> +    int (*enable)(void *ptr); /* Called on every active CPUs */
>>
>> this one instead is "data"
> 
> I committed all the other patches in this series to the my for-4.19 branch

I have left some comments in patch #1. Given this is not the latest 
master, I think we should consider to remove/replace the patch rather 
than introducing a follow-up.

Cheers,
Stefano Stabellini Oct. 17, 2023, 1:03 a.m. UTC | #5
On Mon, 16 Oct 2023, Julien Grall wrote:
> On 14/10/2023 00:34, Stefano Stabellini wrote:
> > On Fri, 13 Oct 2023, Stefano Stabellini wrote:
> > > On Fri, 13 Oct 2023, Federico Serafini wrote:
> > > > Add missing parameter names, no functional change.
> > > > 
> > > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> > > > ---
> > > >   xen/arch/arm/include/asm/cpufeature.h | 8 ++++----
> > > >   1 file changed, 4 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/xen/arch/arm/include/asm/cpufeature.h
> > > > b/xen/arch/arm/include/asm/cpufeature.h
> > > > index 8011076b8c..41e97c23dd 100644
> > > > --- a/xen/arch/arm/include/asm/cpufeature.h
> > > > +++ b/xen/arch/arm/include/asm/cpufeature.h
> > > > @@ -127,8 +127,8 @@ static inline void cpus_set_cap(unsigned int num)
> > > >   struct arm_cpu_capabilities {
> > > >       const char *desc;
> > > >       u16 capability;
> > > > -    bool (*matches)(const struct arm_cpu_capabilities *);
> > > > -    int (*enable)(void *); /* Called on every active CPUs */
> > > > +    bool (*matches)(const struct arm_cpu_capabilities *caps);
> > > 
> > > all the implementations of matches I found in xen/arch/arm/cpuerrata.c
> > > actually call the parameter "entry"
> > > 
> > > 
> > > > +    int (*enable)(void *ptr); /* Called on every active CPUs */
> > > 
> > > this one instead is "data"
> > 
> > I committed all the other patches in this series to the my for-4.19 branch
> 
> I have left some comments in patch #1. Given this is not the latest master, I
> think we should consider to remove/replace the patch rather than introducing a
> follow-up.

Makes sense. I took out patch #1 of this series and also took the
opportunity to rebase 4.19 on the latest staging
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/cpufeature.h b/xen/arch/arm/include/asm/cpufeature.h
index 8011076b8c..41e97c23dd 100644
--- a/xen/arch/arm/include/asm/cpufeature.h
+++ b/xen/arch/arm/include/asm/cpufeature.h
@@ -127,8 +127,8 @@  static inline void cpus_set_cap(unsigned int num)
 struct arm_cpu_capabilities {
     const char *desc;
     u16 capability;
-    bool (*matches)(const struct arm_cpu_capabilities *);
-    int (*enable)(void *); /* Called on every active CPUs */
+    bool (*matches)(const struct arm_cpu_capabilities *caps);
+    int (*enable)(void *ptr); /* Called on every active CPUs */
     union {
         struct {    /* To be used for eratum handling only */
             u32 midr_model;
@@ -448,10 +448,10 @@  struct cpuinfo_arm {
 
 extern struct cpuinfo_arm system_cpuinfo;
 
-extern void identify_cpu(struct cpuinfo_arm *);
+extern void identify_cpu(struct cpuinfo_arm *c);
 
 #ifdef CONFIG_ARM_64
-extern void update_system_features(const struct cpuinfo_arm *);
+extern void update_system_features(const struct cpuinfo_arm *new);
 #else
 static inline void update_system_features(const struct cpuinfo_arm *cpuinfo)
 {