[PATCH 0/2] Address violations of MISRA C:2012 Rule 5.3

[PATCH 0/2] Address violations of MISRA C:2012 Rule 5.3

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 5.3 which states as
following: An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope.

In this series are modified files x86/mm.c and x86/e820.c in which occurred
instances of variable names shadowing a global variable; these patches are aimed
to remove said occurrences leading to partial compliance under MISRA C:2012
Rule 5.3.

No functional change.

Alessandro Zucchelli (2):
  x86/mm address violations of MISRA C:2012 Rule 5.3
  x86/e820 address violations of MISRA C:2012 Rule 5.3

 xen/arch/x86/e820.c | 74 ++++++++++++++++++++++-----------------------
 xen/arch/x86/mm.c   | 12 ++++----
 2 files changed, 43 insertions(+), 43 deletions(-)

-- 
2.34.1

[PATCH 1/2] x86/mm address violations of MISRA C:2012 Rule 5.3

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 5.3 which states as
following: An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/arch/x86/mm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5471b6b1f2..720d56e103 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4703,7 +4703,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     {
         struct xen_foreign_memory_map fmap;
         struct domain *d;
-        struct e820entry *map;
+        struct e820entry *e;
 
         if ( copy_from_guest(&fmap, arg, 1) )
             return -EFAULT;
@@ -4722,23 +4722,23 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
             return rc;
         }
 
-        map = xmalloc_array(e820entry_t, fmap.map.nr_entries);
-        if ( map == NULL )
+        e = xmalloc_array(e820entry_t, fmap.map.nr_entries);
+        if ( e == NULL )
         {
             rcu_unlock_domain(d);
             return -ENOMEM;
         }
 
-        if ( copy_from_guest(map, fmap.map.buffer, fmap.map.nr_entries) )
+        if ( copy_from_guest(e, fmap.map.buffer, fmap.map.nr_entries) )
         {
-            xfree(map);
+            xfree(e);
             rcu_unlock_domain(d);
             return -EFAULT;
         }
 
         spin_lock(&d->arch.e820_lock);
         xfree(d->arch.e820);
-        d->arch.e820 = map;
+        d->arch.e820 = e;
         d->arch.nr_e820 = fmap.map.nr_entries;
         spin_unlock(&d->arch.e820_lock);
 
-- 
2.34.1

Re: [PATCH 1/2] x86/mm address violations of MISRA C:2012 Rule 5.3

[Jan Beulich]

On 14.06.2024 18:12, Alessandro Zucchelli wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4703,7 +4703,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      {
>          struct xen_foreign_memory_map fmap;
>          struct domain *d;
> -        struct e820entry *map;
> +        struct e820entry *e;

What version of the tree is this against? The variable in my copy is named
"e820", and it is only then that I could see what the conflict actually is.
I can't see any conflict with anything named "map". Saying what the actual
conflict is imo also ought to be part if the description.

Jan

[PATCH 2/2] x86/e820 address violations of MISRA C:2012 Rule 5.3

[Alessandro Zucchelli]

This addresses violations of MISRA C:2012 Rule 5.3 which states as
following: An identifier declared in an inner scope shall not hide an
identifier declared in an outer scope.

No functional change.

Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
---
 xen/arch/x86/e820.c | 74 ++++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index 6a3ce7e0a0..3726823e88 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -593,79 +593,79 @@ int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
 }
 
 int __init e820_change_range_type(
-    struct e820map *e820, uint64_t s, uint64_t e,
+    struct e820map *map, uint64_t s, uint64_t e,
     uint32_t orig_type, uint32_t new_type)
 {
     uint64_t rs = 0, re = 0;
     unsigned int i;
 
-    for ( i = 0; i < e820->nr_map; i++ )
+    for ( i = 0; i < map->nr_map; i++ )
     {
         /* Have we found the e820 region that includes the specified range? */
-        rs = e820->map[i].addr;
-        re = rs + e820->map[i].size;
+        rs = map->map[i].addr;
+        re = rs + map->map[i].size;
         if ( (s >= rs) && (e <= re) )
             break;
     }
 
-    if ( (i == e820->nr_map) || (e820->map[i].type != orig_type) )
+    if ( (i == map->nr_map) || (map->map[i].type != orig_type) )
         return 0;
 
     if ( (s == rs) && (e == re) )
     {
-        e820->map[i].type = new_type;
+        map->map[i].type = new_type;
     }
     else if ( (s == rs) || (e == re) )
     {
-        if ( (e820->nr_map + 1) > ARRAY_SIZE(e820->map) )
+        if ( (map->nr_map + 1) > ARRAY_SIZE(map->map) )
             goto overflow;
 
-        memmove(&e820->map[i+1], &e820->map[i],
-                (e820->nr_map-i) * sizeof(e820->map[0]));
-        e820->nr_map++;
+        memmove(&map->map[i+1], &map->map[i],
+                (map->nr_map-i) * sizeof(map->map[0]));
+        map->nr_map++;
 
         if ( s == rs )
         {
-            e820->map[i].size = e - s;
-            e820->map[i].type = new_type;
-            e820->map[i+1].addr = e;
-            e820->map[i+1].size = re - e;
+            map->map[i].size = e - s;
+            map->map[i].type = new_type;
+            map->map[i+1].addr = e;
+            map->map[i+1].size = re - e;
         }
         else
         {
-            e820->map[i].size = s - rs;
-            e820->map[i+1].addr = s;
-            e820->map[i+1].size = e - s;
-            e820->map[i+1].type = new_type;
+            map->map[i].size = s - rs;
+            map->map[i+1].addr = s;
+            map->map[i+1].size = e - s;
+            map->map[i+1].type = new_type;
         }
     }
     else
     {
-        if ( (e820->nr_map + 2) > ARRAY_SIZE(e820->map) )
+        if ( (map->nr_map + 2) > ARRAY_SIZE(map->map) )
             goto overflow;
 
-        memmove(&e820->map[i+2], &e820->map[i],
-                (e820->nr_map-i) * sizeof(e820->map[0]));
-        e820->nr_map += 2;
+        memmove(&map->map[i+2], &map->map[i],
+                (map->nr_map-i) * sizeof(map->map[0]));
+        map->nr_map += 2;
 
-        e820->map[i].size = s - rs;
-        e820->map[i+1].addr = s;
-        e820->map[i+1].size = e - s;
-        e820->map[i+1].type = new_type;
-        e820->map[i+2].addr = e;
-        e820->map[i+2].size = re - e;
+        map->map[i].size = s - rs;
+        map->map[i+1].addr = s;
+        map->map[i+1].size = e - s;
+        map->map[i+1].type = new_type;
+        map->map[i+2].addr = e;
+        map->map[i+2].size = re - e;
     }
 
     /* Finally, look for any opportunities to merge adjacent e820 entries. */
-    for ( i = 0; i < (e820->nr_map - 1); i++ )
+    for ( i = 0; i < (map->nr_map - 1); i++ )
     {
-        if ( (e820->map[i].type != e820->map[i+1].type) ||
-             ((e820->map[i].addr + e820->map[i].size) != e820->map[i+1].addr) )
+        if ( (map->map[i].type != map->map[i+1].type) ||
+             ((map->map[i].addr + map->map[i].size) != map->map[i+1].addr) )
             continue;
-        e820->map[i].size += e820->map[i+1].size;
-        memmove(&e820->map[i+1], &e820->map[i+2],
-                (e820->nr_map-i-2) * sizeof(e820->map[0]));
-        e820->nr_map--;
+        map->map[i].size += map->map[i+1].size;
+        memmove(&map->map[i+1], &map->map[i+2],
+                (map->nr_map-i-2) * sizeof(map->map[0]));
+        map->nr_map--;
         i--;
     }
 
@@ -678,9 +678,9 @@ int __init e820_change_range_type(
 }
 
 /* Set E820_RAM area (@s,@e) as RESERVED in specified e820 map. */
-int __init reserve_e820_ram(struct e820map *e820, uint64_t s, uint64_t e)
+int __init reserve_e820_ram(struct e820map *map, uint64_t s, uint64_t e)
 {
-    return e820_change_range_type(e820, s, e, E820_RAM, E820_RESERVED);
+    return e820_change_range_type(map, s, e, E820_RAM, E820_RESERVED);
 }
 
 unsigned long __init init_e820(const char *str, struct e820map *raw)
-- 
2.34.1

Re: [PATCH 2/2] x86/e820 address violations of MISRA C:2012 Rule 5.3

[Jan Beulich]

On 14.06.2024 18:12, Alessandro Zucchelli wrote:
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -593,79 +593,79 @@ int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
>  }
>  
>  int __init e820_change_range_type(
> -    struct e820map *e820, uint64_t s, uint64_t e,
> +    struct e820map *map, uint64_t s, uint64_t e,
>      uint32_t orig_type, uint32_t new_type)
>  {
>      uint64_t rs = 0, re = 0;
>      unsigned int i;
>  
> -    for ( i = 0; i < e820->nr_map; i++ )
> +    for ( i = 0; i < map->nr_map; i++ )
>      {
>          /* Have we found the e820 region that includes the specified range? */
> -        rs = e820->map[i].addr;
> -        re = rs + e820->map[i].size;
> +        rs = map->map[i].addr;

I'm not overly happy with the many instances of map->map that we're now
gaining, but perhaps that's about as good as it can get. Hence
Acked-by: Jan Beulich <jbeulich@suse.com>

As mentioned for patch 1, please remember though to actually describe
what the conflict is in patches like this one. In this case, unless there
ends up being a need to submit another version, I'll try to remember to
add half a sentence while committing.

Jan