[XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3

Federico Serafini posted 1 patch 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/d70e8e6b00f7b08ed4b360d38113e6a1460ed3ab.1698743361.git.federico.serafini@bugseng.com
xen/arch/arm/domain_page.c    | 10 +++++-----
xen/include/xen/domain_page.h | 12 ++++++------
2 files changed, 11 insertions(+), 11 deletions(-)
[XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3
Posted by Federico Serafini 6 months ago
Make function defintions and declarations consistent.
No functional change.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
Changes in v2:
- use 'ptr' do denote a const void * parameter.
---
 xen/arch/arm/domain_page.c    | 10 +++++-----
 xen/include/xen/domain_page.h | 12 ++++++------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/domain_page.c b/xen/arch/arm/domain_page.c
index b7c02c9190..3a43601623 100644
--- a/xen/arch/arm/domain_page.c
+++ b/xen/arch/arm/domain_page.c
@@ -74,9 +74,9 @@ void *map_domain_page_global(mfn_t mfn)
     return vmap(&mfn, 1);
 }
 
-void unmap_domain_page_global(const void *va)
+void unmap_domain_page_global(const void *ptr)
 {
-    vunmap(va);
+    vunmap(ptr);
 }
 
 /* Map a page of domheap memory */
@@ -149,13 +149,13 @@ void *map_domain_page(mfn_t mfn)
 }
 
 /* Release a mapping taken with map_domain_page() */
-void unmap_domain_page(const void *va)
+void unmap_domain_page(const void *ptr)
 {
     unsigned long flags;
     lpae_t *map = this_cpu(xen_dommap);
-    int slot = ((unsigned long) va - DOMHEAP_VIRT_START) >> SECOND_SHIFT;
+    int slot = ((unsigned long)ptr - DOMHEAP_VIRT_START) >> SECOND_SHIFT;
 
-    if ( !va )
+    if ( !ptr )
         return;
 
     local_irq_save(flags);
diff --git a/xen/include/xen/domain_page.h b/xen/include/xen/domain_page.h
index 0ff5cdd294..e1dd24ae58 100644
--- a/xen/include/xen/domain_page.h
+++ b/xen/include/xen/domain_page.h
@@ -29,12 +29,12 @@ void *map_domain_page(mfn_t mfn);
  * Pass a VA within a page previously mapped in the context of the
  * currently-executing VCPU via a call to map_domain_page().
  */
-void unmap_domain_page(const void *va);
+void unmap_domain_page(const void *ptr);
 
-/* 
+/*
  * Given a VA from map_domain_page(), return its underlying MFN.
  */
-mfn_t domain_page_map_to_mfn(const void *va);
+mfn_t domain_page_map_to_mfn(const void *ptr);
 
 /*
  * Similar to the above calls, except the mapping is accessible in all
@@ -42,7 +42,7 @@ mfn_t domain_page_map_to_mfn(const void *va);
  * mappings can also be unmapped from any context.
  */
 void *map_domain_page_global(mfn_t mfn);
-void unmap_domain_page_global(const void *va);
+void unmap_domain_page_global(const void *ptr);
 
 #define __map_domain_page(pg)        map_domain_page(page_to_mfn(pg))
 
@@ -55,8 +55,8 @@ static inline void *__map_domain_page_global(const struct page_info *pg)
 
 #define map_domain_page(mfn)                __mfn_to_virt(mfn_x(mfn))
 #define __map_domain_page(pg)               page_to_virt(pg)
-#define unmap_domain_page(va)               ((void)(va))
-#define domain_page_map_to_mfn(va)          _mfn(__virt_to_mfn((unsigned long)(va)))
+#define unmap_domain_page(ptr)               ((void)(ptr))
+#define domain_page_map_to_mfn(ptr)          _mfn(__virt_to_mfn((unsigned long)(ptr)))
 
 static inline void *map_domain_page_global(mfn_t mfn)
 {
-- 
2.34.1
Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3
Posted by Jan Beulich 6 months ago
On 31.10.2023 10:25, Federico Serafini wrote:
> Make function defintions and declarations consistent.
> No functional change.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

However, ...

> ---
> Changes in v2:
> - use 'ptr' do denote a const void * parameter.

... not even this (let alone the description) clarifies what the
inconsistency was. I had to go check to figure that x86 already uses
"ptr". Such things could do with spelling out.

> @@ -55,8 +55,8 @@ static inline void *__map_domain_page_global(const struct page_info *pg)
>  
>  #define map_domain_page(mfn)                __mfn_to_virt(mfn_x(mfn))
>  #define __map_domain_page(pg)               page_to_virt(pg)
> -#define unmap_domain_page(va)               ((void)(va))
> -#define domain_page_map_to_mfn(va)          _mfn(__virt_to_mfn((unsigned long)(va)))
> +#define unmap_domain_page(ptr)               ((void)(ptr))
> +#define domain_page_map_to_mfn(ptr)          _mfn(__virt_to_mfn((unsigned long)(ptr)))

Padding wants to not be screwed by the change (one of the blanks will
want dropping). I guess this can be taken care of while committing.

Jan
Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3
Posted by Julien Grall 6 months ago
Hi,

On 31/10/2023 10:31, Jan Beulich wrote:
> On 31.10.2023 10:25, Federico Serafini wrote:
>> Make function defintions and declarations consistent.

typo: s/defintions/definitions/

>> No functional change.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> Acked-by: Jan Beulich <jbeulich@suse.com>
> 
> However, ...
> 
>> ---
>> Changes in v2:
>> - use 'ptr' do denote a const void * parameter.
> 
> ... not even this (let alone the description) clarifies what the
> inconsistency was. I had to go check to figure that x86 already uses
> "ptr". Such things could do with spelling out.

+1. The more that x86 was the "odd" one but it was chosen to use the 
variant everywhere.

With the commit message clarified:

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,

> 
>> @@ -55,8 +55,8 @@ static inline void *__map_domain_page_global(const struct page_info *pg)
>>   
>>   #define map_domain_page(mfn)                __mfn_to_virt(mfn_x(mfn))
>>   #define __map_domain_page(pg)               page_to_virt(pg)
>> -#define unmap_domain_page(va)               ((void)(va))
>> -#define domain_page_map_to_mfn(va)          _mfn(__virt_to_mfn((unsigned long)(va)))
>> +#define unmap_domain_page(ptr)               ((void)(ptr))
>> +#define domain_page_map_to_mfn(ptr)          _mfn(__virt_to_mfn((unsigned long)(ptr)))
> 
> Padding wants to not be screwed by the change (one of the blanks will
> want dropping). I guess this can be taken care of while committing.
> 
> Jan

-- 
Julien Grall
Re: [XEN PATCH v2] xen/domain_page: address violations of MISRA C:2012 Rule 8.3
Posted by Stefano Stabellini 6 months ago
On Tue, 31 Oct 2023, Julien Grall wrote:
> Hi,
> 
> On 31/10/2023 10:31, Jan Beulich wrote:
> > On 31.10.2023 10:25, Federico Serafini wrote:
> > > Make function defintions and declarations consistent.
> 
> typo: s/defintions/definitions/
> 
> > > No functional change.
> > > 
> > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> > 
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > 
> > However, ...
> > 
> > > ---
> > > Changes in v2:
> > > - use 'ptr' do denote a const void * parameter.
> > 
> > ... not even this (let alone the description) clarifies what the
> > inconsistency was. I had to go check to figure that x86 already uses
> > "ptr". Such things could do with spelling out.
> 
> +1. The more that x86 was the "odd" one but it was chosen to use the variant
> everywhere.
> 
> With the commit message clarified:
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

I made the changes on commit to my for-4.19 branch (I am going to wait
until staging fully reopens before moving commits to staging).