* [XEN PATCH v2 1/5] x86: address MISRA C:2012 Rule 5.3
2023-08-08 11:08 [XEN PATCH v2 0/5] x86: address MISRA C:2012 Rule 5.3 Nicola Vetrini
@ 2023-08-08 11:08 ` Nicola Vetrini
2023-08-08 13:46 ` Jan Beulich
2023-08-08 11:08 ` [XEN PATCH v2 2/5] xen/delay: " Nicola Vetrini
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 11:08 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Nicola Vetrini, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu, Paul Durrant
Address some occurrences of shadowing between the global
variable 'e820' in 'xen/arch/x86/e820.c' and function
parameter names (such as that of 'e820_add_range').
The parameter is removed in those functions whose call chain
ultimately supplies the global variable as a parameter, which
is already visible from their definitions in 'e820.c'.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- Reworked the patch to remove the parameter e820 where possible
and consequential changes to the involved functions.
---
xen/arch/x86/e820.c | 36 ++++++++++-----------
xen/arch/x86/guest/hyperv/hyperv.c | 4 +--
xen/arch/x86/guest/hypervisor.c | 2 +-
xen/arch/x86/guest/xen/xen.c | 4 +--
xen/arch/x86/include/asm/e820.h | 3 +-
xen/arch/x86/include/asm/guest/hypervisor.h | 2 +-
xen/arch/x86/include/asm/pv/shim.h | 2 +-
xen/arch/x86/pv/shim.c | 10 +++---
xen/arch/x86/setup.c | 8 ++---
xen/arch/x86/x86_64/mmconf-fam10h.c | 2 +-
xen/drivers/passthrough/amd/iommu_acpi.c | 2 +-
11 files changed, 37 insertions(+), 38 deletions(-)
diff --git a/xen/arch/x86/e820.c b/xen/arch/x86/e820.c
index 4911e64b8c..875572b23e 100644
--- a/xen/arch/x86/e820.c
+++ b/xen/arch/x86/e820.c
@@ -543,27 +543,27 @@ static void __init machine_specific_memory_setup(struct e820map *raw)
clip_to_limit(top_of_ram, "MTRRs do not cover all of memory.");
}
-/* This function relies on the passed in e820->map[] being sorted. */
-int __init e820_add_range(
- struct e820map *e820, uint64_t s, uint64_t e, uint32_t type)
+/* This function relies on the global e820->map[] being sorted. */
+int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
{
unsigned int i;
+ struct e820entry *ei = e820.map;
- for ( i = 0; i < e820->nr_map; ++i )
+ for ( i = 0; i < e820.nr_map; ++i )
{
- uint64_t rs = e820->map[i].addr;
- uint64_t re = rs + e820->map[i].size;
+ uint64_t rs = ei[i].addr;
+ uint64_t re = rs + ei[i].size;
- if ( rs == e && e820->map[i].type == type )
+ if ( rs == e && ei[i].type == type )
{
- e820->map[i].addr = s;
+ ei[i].addr = s;
return 1;
}
- if ( re == s && e820->map[i].type == type &&
- (i + 1 == e820->nr_map || e820->map[i + 1].addr >= e) )
+ if ( re == s && ei[i].type == type &&
+ (i + 1 == e820.nr_map || ei[i + 1].addr >= e) )
{
- e820->map[i].size += e - s;
+ ei[i].size += e - s;
return 1;
}
@@ -574,20 +574,20 @@ int __init e820_add_range(
return 0;
}
- if ( e820->nr_map >= ARRAY_SIZE(e820->map) )
+ if ( e820.nr_map >= ARRAY_SIZE(e820.map) )
{
printk(XENLOG_WARNING "E820: overflow while adding region"
" %"PRIx64"-%"PRIx64"\n", s, e);
return 0;
}
- memmove(e820->map + i + 1, e820->map + i,
- (e820->nr_map - i) * sizeof(*e820->map));
+ memmove(ei + i + 1, ei + i,
+ (e820.nr_map - i) * sizeof(*e820.map));
- e820->nr_map++;
- e820->map[i].addr = s;
- e820->map[i].size = e - s;
- e820->map[i].type = type;
+ e820.nr_map++;
+ ei[i].addr = s;
+ ei[i].size = e - s;
+ ei[i].type = type;
return 1;
}
diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
index aacc7a6167..912099564e 100644
--- a/xen/arch/x86/guest/hyperv/hyperv.c
+++ b/xen/arch/x86/guest/hyperv/hyperv.c
@@ -187,11 +187,11 @@ static int cf_check ap_setup(void)
return setup_vp_assist();
}
-static void __init cf_check e820_fixup(struct e820map *e820)
+static void __init cf_check e820_fixup(void)
{
uint64_t s = HV_HCALL_MFN << PAGE_SHIFT;
- if ( !e820_add_range(e820, s, s + PAGE_SIZE, E820_RESERVED) )
+ if ( !e820_add_range(s, s + PAGE_SIZE, E820_RESERVED) )
panic("Unable to reserve Hyper-V hypercall range\n");
}
diff --git a/xen/arch/x86/guest/hypervisor.c b/xen/arch/x86/guest/hypervisor.c
index b8549a131a..bf5be97bc3 100644
--- a/xen/arch/x86/guest/hypervisor.c
+++ b/xen/arch/x86/guest/hypervisor.c
@@ -63,7 +63,7 @@ void hypervisor_resume(void)
void __init hypervisor_e820_fixup(struct e820map *e820)
{
if ( ops.e820_fixup )
- ops.e820_fixup(e820);
+ ops.e820_fixup();
}
int hypervisor_flush_tlb(const cpumask_t *mask, const void *va,
diff --git a/xen/arch/x86/guest/xen/xen.c b/xen/arch/x86/guest/xen/xen.c
index f93dfc89f7..139489c666 100644
--- a/xen/arch/x86/guest/xen/xen.c
+++ b/xen/arch/x86/guest/xen/xen.c
@@ -306,10 +306,10 @@ static void cf_check resume(void)
pv_console_init();
}
-static void __init cf_check e820_fixup(struct e820map *e820)
+static void __init cf_check e820_fixup()
{
if ( pv_shim )
- pv_shim_fixup_e820(e820);
+ pv_shim_fixup_e820();
}
static int cf_check flush_tlb(
diff --git a/xen/arch/x86/include/asm/e820.h b/xen/arch/x86/include/asm/e820.h
index 213d5b5dd2..af90085d65 100644
--- a/xen/arch/x86/include/asm/e820.h
+++ b/xen/arch/x86/include/asm/e820.h
@@ -29,8 +29,7 @@ extern int reserve_e820_ram(struct e820map *e820, uint64_t s, uint64_t e);
extern int e820_change_range_type(
struct e820map *e820, uint64_t s, uint64_t e,
uint32_t orig_type, uint32_t new_type);
-extern int e820_add_range(
- struct e820map *, uint64_t s, uint64_t e, uint32_t type);
+extern int e820_add_range(uint64_t s, uint64_t e, uint32_t type);
extern unsigned long init_e820(const char *str, struct e820map *raw);
extern void print_e820_memory_map(const struct e820entry *map,
unsigned int entries);
diff --git a/xen/arch/x86/include/asm/guest/hypervisor.h b/xen/arch/x86/include/asm/guest/hypervisor.h
index 4cffea3866..9c8a893da3 100644
--- a/xen/arch/x86/include/asm/guest/hypervisor.h
+++ b/xen/arch/x86/include/asm/guest/hypervisor.h
@@ -22,7 +22,7 @@ struct hypervisor_ops {
/* Resume from suspension */
void (*resume)(void);
/* Fix up e820 map */
- void (*e820_fixup)(struct e820map *e820);
+ void (*e820_fixup)(void);
/* L0 assisted TLB flush */
int (*flush_tlb)(const cpumask_t *mask, const void *va, unsigned int flags);
};
diff --git a/xen/arch/x86/include/asm/pv/shim.h b/xen/arch/x86/include/asm/pv/shim.h
index 5625b90b72..7bae9ae372 100644
--- a/xen/arch/x86/include/asm/pv/shim.h
+++ b/xen/arch/x86/include/asm/pv/shim.h
@@ -85,7 +85,7 @@ static inline uint64_t pv_shim_mem(uint64_t avail)
ASSERT_UNREACHABLE();
return 0;
}
-static inline void pv_shim_fixup_e820(struct e820map *e820)
+static inline void pv_shim_fixup_e820(void)
{
ASSERT_UNREACHABLE();
}
diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
index 4044087119..a8883a1ebd 100644
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -98,17 +98,17 @@ uint64_t pv_shim_mem(uint64_t avail)
return shim_nrpages;
}
-static void __init mark_pfn_as_ram(struct e820map *e820, uint64_t pfn)
+static void __init mark_pfn_as_ram(uint64_t pfn)
{
- if ( !e820_add_range(e820, pfn << PAGE_SHIFT,
+ if ( !e820_add_range(pfn << PAGE_SHIFT,
(pfn << PAGE_SHIFT) + PAGE_SIZE, E820_RAM) &&
- !e820_change_range_type(e820, pfn << PAGE_SHIFT,
+ !e820_change_range_type(&e820, pfn << PAGE_SHIFT,
(pfn << PAGE_SHIFT) + PAGE_SIZE,
E820_RESERVED, E820_RAM) )
panic("Unable to add/change memory type of pfn %#lx to RAM\n", pfn);
}
-void __init pv_shim_fixup_e820(struct e820map *e820)
+void __init pv_shim_fixup_e820(void)
{
uint64_t pfn = 0;
unsigned int i = 0;
@@ -120,7 +120,7 @@ void __init pv_shim_fixup_e820(struct e820map *e820)
rc = xen_hypercall_hvm_get_param(p, &pfn); \
if ( rc ) \
panic("Unable to get " #p "\n"); \
- mark_pfn_as_ram(e820, pfn); \
+ mark_pfn_as_ram(pfn); \
ASSERT(i < ARRAY_SIZE(reserved_pages)); \
reserved_pages[i++].mfn = pfn; \
})
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 80ae973d64..03f9a03180 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -686,7 +686,7 @@ static void __init parse_video_info(void)
#endif
}
-static void __init kexec_reserve_area(struct e820map *e820)
+static void __init kexec_reserve_area(void)
{
#ifdef CONFIG_KEXEC
unsigned long kdump_start = kexec_crash_area.start;
@@ -700,7 +700,7 @@ static void __init kexec_reserve_area(struct e820map *e820)
is_reserved = true;
- if ( !reserve_e820_ram(e820, kdump_start, kdump_start + kdump_size) )
+ if ( !reserve_e820_ram(&boot_e820, kdump_start, kdump_start + kdump_size) )
{
printk("Kdump: DISABLED (failed to reserve %luMB (%lukB) at %#lx)"
"\n", kdump_size >> 20, kdump_size >> 10, kdump_start);
@@ -1308,7 +1308,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
if ( e820.map[i].type == E820_RAM )
nr_pages += e820.map[i].size >> PAGE_SHIFT;
set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
- kexec_reserve_area(&boot_e820);
+ kexec_reserve_area();
initial_images = mod;
nr_initial_images = mbi->mods_count;
@@ -1495,7 +1495,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
reserve_e820_ram(&boot_e820, __pa(_stext), __pa(__2M_rwdata_end));
/* Late kexec reservation (dynamic start address). */
- kexec_reserve_area(&boot_e820);
+ kexec_reserve_area();
setup_max_pdx(raw_max_page);
if ( highmem_start )
diff --git a/xen/arch/x86/x86_64/mmconf-fam10h.c b/xen/arch/x86/x86_64/mmconf-fam10h.c
index a834ab3149..36b32eb769 100644
--- a/xen/arch/x86/x86_64/mmconf-fam10h.c
+++ b/xen/arch/x86/x86_64/mmconf-fam10h.c
@@ -135,7 +135,7 @@ static void __init get_fam10h_pci_mmconf_base(void)
return;
out:
- if (e820_add_range(&e820, start, start + SIZE, E820_RESERVED))
+ if (e820_add_range(start, start + SIZE, E820_RESERVED))
fam10h_pci_mmconf_base = start;
}
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c
index 3b577c9b39..db993d6df2 100644
--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -418,7 +418,7 @@ static int __init parse_ivmd_block(const struct acpi_ivrs_memory *ivmd_block)
if ( type == RAM_TYPE_UNKNOWN )
{
- if ( e820_add_range(&e820, addr, addr + PAGE_SIZE,
+ if ( e820_add_range(addr, addr + PAGE_SIZE,
E820_RESERVED) )
continue;
AMD_IOMMU_ERROR("IVMD: page at %lx couldn't be reserved\n",
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 1/5] x86: address MISRA C:2012 Rule 5.3
2023-08-08 11:08 ` [XEN PATCH v2 1/5] " Nicola Vetrini
@ 2023-08-08 13:46 ` Jan Beulich
2023-08-09 7:32 ` Nicola Vetrini
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 13:46 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
Paul Durrant, xen-devel
On 08.08.2023 13:08, Nicola Vetrini wrote:
> --- a/xen/arch/x86/e820.c
> +++ b/xen/arch/x86/e820.c
> @@ -543,27 +543,27 @@ static void __init machine_specific_memory_setup(struct e820map *raw)
> clip_to_limit(top_of_ram, "MTRRs do not cover all of memory.");
> }
>
> -/* This function relies on the passed in e820->map[] being sorted. */
> -int __init e820_add_range(
> - struct e820map *e820, uint64_t s, uint64_t e, uint32_t type)
> +/* This function relies on the global e820->map[] being sorted. */
> +int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
> {
> unsigned int i;
> + struct e820entry *ei = e820.map;
>
> - for ( i = 0; i < e820->nr_map; ++i )
> + for ( i = 0; i < e820.nr_map; ++i )
> {
> - uint64_t rs = e820->map[i].addr;
> - uint64_t re = rs + e820->map[i].size;
> + uint64_t rs = ei[i].addr;
> + uint64_t re = rs + ei[i].size;
>
> - if ( rs == e && e820->map[i].type == type )
> + if ( rs == e && ei[i].type == type )
> {
> - e820->map[i].addr = s;
> + ei[i].addr = s;
> return 1;
> }
>
> - if ( re == s && e820->map[i].type == type &&
> - (i + 1 == e820->nr_map || e820->map[i + 1].addr >= e) )
> + if ( re == s && ei[i].type == type &&
> + (i + 1 == e820.nr_map || ei[i + 1].addr >= e) )
> {
> - e820->map[i].size += e - s;
> + ei[i].size += e - s;
> return 1;
> }
>
> @@ -574,20 +574,20 @@ int __init e820_add_range(
> return 0;
> }
>
> - if ( e820->nr_map >= ARRAY_SIZE(e820->map) )
> + if ( e820.nr_map >= ARRAY_SIZE(e820.map) )
> {
> printk(XENLOG_WARNING "E820: overflow while adding region"
> " %"PRIx64"-%"PRIx64"\n", s, e);
> return 0;
> }
>
> - memmove(e820->map + i + 1, e820->map + i,
> - (e820->nr_map - i) * sizeof(*e820->map));
> + memmove(ei + i + 1, ei + i,
> + (e820.nr_map - i) * sizeof(*e820.map));
>
> - e820->nr_map++;
> - e820->map[i].addr = s;
> - e820->map[i].size = e - s;
> - e820->map[i].type = type;
> + e820.nr_map++;
> + ei[i].addr = s;
> + ei[i].size = e - s;
> + ei[i].type = type;
>
> return 1;
> }
To be honest this isn't quite what I was hoping for; the many ei[i]. are
(imo) quite a bit harder to read than ei-> would have been (hence my
earlier suggestion to also update that pointer in the for() loop header).
Then again I see there is one use of ei[i + 1], which would likely look
less neat as ei[1].addr when everywhere else we have ei->. So I guess up
to you whether you adjust further; I'll ack either form.
> --- a/xen/arch/x86/guest/hypervisor.c
> +++ b/xen/arch/x86/guest/hypervisor.c
> @@ -63,7 +63,7 @@ void hypervisor_resume(void)
> void __init hypervisor_e820_fixup(struct e820map *e820)
What about this one? The function parameter ...
> {
> if ( ops.e820_fixup )
> - ops.e820_fixup(e820);
> + ops.e820_fixup();
> }
... isn't used anymore, and the sole call site passes &e820.
> --- a/xen/arch/x86/include/asm/e820.h
> +++ b/xen/arch/x86/include/asm/e820.h
> @@ -29,8 +29,7 @@ extern int reserve_e820_ram(struct e820map *e820, uint64_t s, uint64_t e);
> extern int e820_change_range_type(
> struct e820map *e820, uint64_t s, uint64_t e,
> uint32_t orig_type, uint32_t new_type);
And what about this one? None of the other subjects in the series suggest
this is then taken care of in a separate patch (as per the earlier
discussion it indeed doesn't want dealing with right here).
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -686,7 +686,7 @@ static void __init parse_video_info(void)
> #endif
> }
>
> -static void __init kexec_reserve_area(struct e820map *e820)
> +static void __init kexec_reserve_area(void)
> {
> #ifdef CONFIG_KEXEC
> unsigned long kdump_start = kexec_crash_area.start;
> @@ -700,7 +700,7 @@ static void __init kexec_reserve_area(struct e820map *e820)
>
> is_reserved = true;
>
> - if ( !reserve_e820_ram(e820, kdump_start, kdump_start + kdump_size) )
> + if ( !reserve_e820_ram(&boot_e820, kdump_start, kdump_start + kdump_size) )
> {
> printk("Kdump: DISABLED (failed to reserve %luMB (%lukB) at %#lx)"
> "\n", kdump_size >> 20, kdump_size >> 10, kdump_start);
> @@ -1308,7 +1308,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> if ( e820.map[i].type == E820_RAM )
> nr_pages += e820.map[i].size >> PAGE_SHIFT;
> set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
> - kexec_reserve_area(&boot_e820);
> + kexec_reserve_area();
>
> initial_images = mod;
> nr_initial_images = mbi->mods_count;
> @@ -1495,7 +1495,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
> reserve_e820_ram(&boot_e820, __pa(_stext), __pa(__2M_rwdata_end));
>
> /* Late kexec reservation (dynamic start address). */
> - kexec_reserve_area(&boot_e820);
> + kexec_reserve_area();
>
> setup_max_pdx(raw_max_page);
> if ( highmem_start )
Seeing all the knock-on effects for the add_range() change, I think this
separate adjustment would better have been an independent patch.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 1/5] x86: address MISRA C:2012 Rule 5.3
2023-08-08 13:46 ` Jan Beulich
@ 2023-08-09 7:32 ` Nicola Vetrini
0 siblings, 0 replies; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-09 7:32 UTC (permalink / raw)
To: Jan Beulich
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
Paul Durrant, xen-devel
On 08/08/2023 15:46, Jan Beulich wrote:
> On 08.08.2023 13:08, Nicola Vetrini wrote:
>> --- a/xen/arch/x86/e820.c
>> +++ b/xen/arch/x86/e820.c
>> @@ -543,27 +543,27 @@ static void __init
>> machine_specific_memory_setup(struct e820map *raw)
>> clip_to_limit(top_of_ram, "MTRRs do not cover all of
>> memory.");
>> }
>>
>> -/* This function relies on the passed in e820->map[] being sorted. */
>> -int __init e820_add_range(
>> - struct e820map *e820, uint64_t s, uint64_t e, uint32_t type)
>> +/* This function relies on the global e820->map[] being sorted. */
>> +int __init e820_add_range(uint64_t s, uint64_t e, uint32_t type)
>> {
>> unsigned int i;
>> + struct e820entry *ei = e820.map;
>>
>> - for ( i = 0; i < e820->nr_map; ++i )
>> + for ( i = 0; i < e820.nr_map; ++i )
>> {
>> - uint64_t rs = e820->map[i].addr;
>> - uint64_t re = rs + e820->map[i].size;
>> + uint64_t rs = ei[i].addr;
>> + uint64_t re = rs + ei[i].size;
>>
>> - if ( rs == e && e820->map[i].type == type )
>> + if ( rs == e && ei[i].type == type )
>> {
>> - e820->map[i].addr = s;
>> + ei[i].addr = s;
>> return 1;
>> }
>>
>> - if ( re == s && e820->map[i].type == type &&
>> - (i + 1 == e820->nr_map || e820->map[i + 1].addr >= e) )
>> + if ( re == s && ei[i].type == type &&
>> + (i + 1 == e820.nr_map || ei[i + 1].addr >= e) )
>> {
>> - e820->map[i].size += e - s;
>> + ei[i].size += e - s;
>> return 1;
>> }
>>
>> @@ -574,20 +574,20 @@ int __init e820_add_range(
>> return 0;
>> }
>>
>> - if ( e820->nr_map >= ARRAY_SIZE(e820->map) )
>> + if ( e820.nr_map >= ARRAY_SIZE(e820.map) )
>> {
>> printk(XENLOG_WARNING "E820: overflow while adding region"
>> " %"PRIx64"-%"PRIx64"\n", s, e);
>> return 0;
>> }
>>
>> - memmove(e820->map + i + 1, e820->map + i,
>> - (e820->nr_map - i) * sizeof(*e820->map));
>> + memmove(ei + i + 1, ei + i,
>> + (e820.nr_map - i) * sizeof(*e820.map));
>>
>> - e820->nr_map++;
>> - e820->map[i].addr = s;
>> - e820->map[i].size = e - s;
>> - e820->map[i].type = type;
>> + e820.nr_map++;
>> + ei[i].addr = s;
>> + ei[i].size = e - s;
>> + ei[i].type = type;
>>
>> return 1;
>> }
>
> To be honest this isn't quite what I was hoping for; the many ei[i].
> are
> (imo) quite a bit harder to read than ei-> would have been (hence my
> earlier suggestion to also update that pointer in the for() loop
> header).
> Then again I see there is one use of ei[i + 1], which would likely look
> less neat as ei[1].addr when everywhere else we have ei->. So I guess
> up
> to you whether you adjust further; I'll ack either form.
>
I'll leave it as is.
>> --- a/xen/arch/x86/guest/hypervisor.c
>> +++ b/xen/arch/x86/guest/hypervisor.c
>> @@ -63,7 +63,7 @@ void hypervisor_resume(void)
>> void __init hypervisor_e820_fixup(struct e820map *e820)
>
> What about this one? The function parameter ...
>
>> {
>> if ( ops.e820_fixup )
>> - ops.e820_fixup(e820);
>> + ops.e820_fixup();
>> }
>
> ... isn't used anymore, and the sole call site passes &e820.
>
It remained there by accident.
>> --- a/xen/arch/x86/include/asm/e820.h
>> +++ b/xen/arch/x86/include/asm/e820.h
>> @@ -29,8 +29,7 @@ extern int reserve_e820_ram(struct e820map *e820,
>> uint64_t s, uint64_t e);
>> extern int e820_change_range_type(
>> struct e820map *e820, uint64_t s, uint64_t e,
>> uint32_t orig_type, uint32_t new_type);
>
> And what about this one? None of the other subjects in the series
> suggest
> this is then taken care of in a separate patch (as per the earlier
> discussion it indeed doesn't want dealing with right here).
>
I'll mention this detail. While I work on other rules I'll think of a
good way to rename.
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -686,7 +686,7 @@ static void __init parse_video_info(void)
>> #endif
>> }
>>
>> -static void __init kexec_reserve_area(struct e820map *e820)
>> +static void __init kexec_reserve_area(void)
>> {
>> #ifdef CONFIG_KEXEC
>> unsigned long kdump_start = kexec_crash_area.start;
>> @@ -700,7 +700,7 @@ static void __init kexec_reserve_area(struct
>> e820map *e820)
>>
>> is_reserved = true;
>>
>> - if ( !reserve_e820_ram(e820, kdump_start, kdump_start +
>> kdump_size) )
>> + if ( !reserve_e820_ram(&boot_e820, kdump_start, kdump_start +
>> kdump_size) )
>> {
>> printk("Kdump: DISABLED (failed to reserve %luMB (%lukB) at
>> %#lx)"
>> "\n", kdump_size >> 20, kdump_size >> 10,
>> kdump_start);
>> @@ -1308,7 +1308,7 @@ void __init noreturn __start_xen(unsigned long
>> mbi_p)
>> if ( e820.map[i].type == E820_RAM )
>> nr_pages += e820.map[i].size >> PAGE_SHIFT;
>> set_kexec_crash_area_size((u64)nr_pages << PAGE_SHIFT);
>> - kexec_reserve_area(&boot_e820);
>> + kexec_reserve_area();
>>
>> initial_images = mod;
>> nr_initial_images = mbi->mods_count;
>> @@ -1495,7 +1495,7 @@ void __init noreturn __start_xen(unsigned long
>> mbi_p)
>> reserve_e820_ram(&boot_e820, __pa(_stext),
>> __pa(__2M_rwdata_end));
>>
>> /* Late kexec reservation (dynamic start address). */
>> - kexec_reserve_area(&boot_e820);
>> + kexec_reserve_area();
>>
>> setup_max_pdx(raw_max_page);
>> if ( highmem_start )
>
> Seeing all the knock-on effects for the add_range() change, I think
> this
> separate adjustment would better have been an independent patch.
>
> Jan
I can submit it standalone and put together x86/vmsi and delay
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [XEN PATCH v2 2/5] xen/delay: address MISRA C:2012 Rule 5.3.
2023-08-08 11:08 [XEN PATCH v2 0/5] x86: address MISRA C:2012 Rule 5.3 Nicola Vetrini
2023-08-08 11:08 ` [XEN PATCH v2 1/5] " Nicola Vetrini
@ 2023-08-08 11:08 ` Nicola Vetrini
2023-08-08 13:56 ` Jan Beulich
2023-08-08 11:08 ` [XEN PATCH v2 3/5] x86/include: " Nicola Vetrini
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 11:08 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Nicola Vetrini, Andrew Cooper, George Dunlap,
Jan Beulich, Julien Grall, Wei Liu
The variable 'msec' declared in the macro shadows the local
variable in 'ehci_dbgp_bios_handoff', but to prevent any
future clashes with other functions the macro is converted to
a static inline function.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- Switched to a static inline function, as suggested by Julien
---
xen/include/xen/delay.h | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/xen/include/xen/delay.h b/xen/include/xen/delay.h
index 9d70ef035f..6ecee851f6 100644
--- a/xen/include/xen/delay.h
+++ b/xen/include/xen/delay.h
@@ -4,7 +4,12 @@
/* Copyright (C) 1993 Linus Torvalds */
#include <asm/delay.h>
-#define mdelay(n) (\
- {unsigned long msec=(n); while (msec--) udelay(1000);})
+
+static inline void mdelay(unsigned long n)
+{
+ unsigned long msec=n;
+ while ( msec-- )
+ udelay(1000);
+}
#endif /* defined(_LINUX_DELAY_H) */
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 2/5] xen/delay: address MISRA C:2012 Rule 5.3.
2023-08-08 11:08 ` [XEN PATCH v2 2/5] xen/delay: " Nicola Vetrini
@ 2023-08-08 13:56 ` Jan Beulich
2023-08-08 13:57 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 13:56 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu,
xen-devel
On 08.08.2023 13:08, Nicola Vetrini wrote:
> --- a/xen/include/xen/delay.h
> +++ b/xen/include/xen/delay.h
> @@ -4,7 +4,12 @@
> /* Copyright (C) 1993 Linus Torvalds */
>
> #include <asm/delay.h>
> -#define mdelay(n) (\
> - {unsigned long msec=(n); while (msec--) udelay(1000);})
> +
> +static inline void mdelay(unsigned long n)
> +{
> + unsigned long msec=n;
> + while ( msec-- )
> + udelay(1000);
> +}
Nit: Style (blanks around = and blank line between declaration and
statement). If need be I guess this again can be taken care of while
committing. With the adjustments
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 2/5] xen/delay: address MISRA C:2012 Rule 5.3.
2023-08-08 13:56 ` Jan Beulich
@ 2023-08-08 13:57 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 13:57 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, George Dunlap, Julien Grall, Wei Liu,
xen-devel
On 08.08.2023 15:56, Jan Beulich wrote:
> On 08.08.2023 13:08, Nicola Vetrini wrote:
>> --- a/xen/include/xen/delay.h
>> +++ b/xen/include/xen/delay.h
>> @@ -4,7 +4,12 @@
>> /* Copyright (C) 1993 Linus Torvalds */
>>
>> #include <asm/delay.h>
>> -#define mdelay(n) (\
>> - {unsigned long msec=(n); while (msec--) udelay(1000);})
>> +
>> +static inline void mdelay(unsigned long n)
>> +{
>> + unsigned long msec=n;
>> + while ( msec-- )
>> + udelay(1000);
>> +}
>
> Nit: Style (blanks around = and blank line between declaration and
> statement). If need be I guess this again can be taken care of while
> committing. With the adjustments
> Acked-by: Jan Beulich <jbeulich@suse.com>
Actually - why have a local variable here at all? Just name the
function parameter "msec".
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [XEN PATCH v2 3/5] x86/include: address MISRA C:2012 Rule 5.3.
2023-08-08 11:08 [XEN PATCH v2 0/5] x86: address MISRA C:2012 Rule 5.3 Nicola Vetrini
2023-08-08 11:08 ` [XEN PATCH v2 1/5] " Nicola Vetrini
2023-08-08 11:08 ` [XEN PATCH v2 2/5] xen/delay: " Nicola Vetrini
@ 2023-08-08 11:08 ` Nicola Vetrini
2023-08-08 14:01 ` Jan Beulich
2023-08-08 11:08 ` [XEN PATCH v2 4/5] x86/xstate: " Nicola Vetrini
2023-08-08 11:08 ` [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h' Nicola Vetrini
4 siblings, 1 reply; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 11:08 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Nicola Vetrini, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu
Variable 'mpc_default_type' in 'xen/arch/x86/include/asm/mpspec.h'
has no uses and causes shadowing with function parameter names
in 'mpparse.c'. Therefore, it is removed.
No functional changes.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- Removed the variable.
---
xen/arch/x86/include/asm/mpspec.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/xen/arch/x86/include/asm/mpspec.h b/xen/arch/x86/include/asm/mpspec.h
index 1246eece0b..45e474dfd1 100644
--- a/xen/arch/x86/include/asm/mpspec.h
+++ b/xen/arch/x86/include/asm/mpspec.h
@@ -15,7 +15,6 @@ extern void get_smp_config (void);
extern unsigned char apic_version [MAX_APICS];
extern int mp_irq_entries;
extern struct mpc_config_intsrc mp_irqs [MAX_IRQ_SOURCES];
-extern int mpc_default_type;
extern unsigned long mp_lapic_addr;
extern bool pic_mode;
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [XEN PATCH v2 3/5] x86/include: address MISRA C:2012 Rule 5.3.
2023-08-08 11:08 ` [XEN PATCH v2 3/5] x86/include: " Nicola Vetrini
@ 2023-08-08 14:01 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 14:01 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
xen-devel
On 08.08.2023 13:08, Nicola Vetrini wrote:
> Variable 'mpc_default_type' in 'xen/arch/x86/include/asm/mpspec.h'
> has no uses and causes shadowing with function parameter names
> in 'mpparse.c'. Therefore, it is removed.
Personally I'd again wish this could be expressed more precisely than
just "variable". What you're removing is a declaration which has no
definition. So aiui shadowing is only one of the concerns Misra would
have here; the lack of a definition would be another.
> No functional changes.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [XEN PATCH v2 4/5] x86/xstate: address MISRA C:2012 Rule 5.3
2023-08-08 11:08 [XEN PATCH v2 0/5] x86: address MISRA C:2012 Rule 5.3 Nicola Vetrini
` (2 preceding siblings ...)
2023-08-08 11:08 ` [XEN PATCH v2 3/5] x86/include: " Nicola Vetrini
@ 2023-08-08 11:08 ` Nicola Vetrini
2023-08-08 14:03 ` Jan Beulich
2023-08-08 11:08 ` [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h' Nicola Vetrini
4 siblings, 1 reply; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 11:08 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Nicola Vetrini, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu
Rename the local variables s/xsave/xstate/ to avoid clashing
with function 'xsave' defined in 'xen/arch/x86/include/asm/xstate.h'.
No functional changes.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- s/xsave_area/xstate/, since that is the newest common name, also
used in other parts of the modified file.
---
xen/arch/x86/xstate.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 770747d88f..f442610fc5 100644
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -174,10 +174,10 @@ static void setup_xstate_comp(uint16_t *comp_offsets,
*/
void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
{
- const struct xsave_struct *xsave = v->arch.xsave_area;
+ const struct xsave_struct *xstate = v->arch.xsave_area;
const void *src;
uint16_t comp_offsets[sizeof(xfeature_mask)*8];
- u64 xstate_bv = xsave->xsave_hdr.xstate_bv;
+ u64 xstate_bv = xstate->xsave_hdr.xstate_bv;
u64 valid;
/* Check there is state to serialise (i.e. at least an XSAVE_HDR) */
@@ -185,19 +185,19 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
/* Check there is the correct room to decompress into. */
BUG_ON(size != xstate_ctxt_size(v->arch.xcr0_accum));
- if ( !(xsave->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) )
+ if ( !(xstate->xsave_hdr.xcomp_bv & XSTATE_COMPACTION_ENABLED) )
{
- memcpy(dest, xsave, size);
+ memcpy(dest, xstate, size);
return;
}
- ASSERT(xsave_area_compressed(xsave));
- setup_xstate_comp(comp_offsets, xsave->xsave_hdr.xcomp_bv);
+ ASSERT(xsave_area_compressed(xstate));
+ setup_xstate_comp(comp_offsets, xstate->xsave_hdr.xcomp_bv);
/*
* Copy legacy XSAVE area and XSAVE hdr area.
*/
- memcpy(dest, xsave, XSTATE_AREA_MIN_SIZE);
+ memcpy(dest, xstate, XSTATE_AREA_MIN_SIZE);
memset(dest + XSTATE_AREA_MIN_SIZE, 0, size - XSTATE_AREA_MIN_SIZE);
((struct xsave_struct *)dest)->xsave_hdr.xcomp_bv = 0;
@@ -206,7 +206,7 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
* Copy each region from the possibly compacted offset to the
* non-compacted offset.
*/
- src = xsave;
+ src = xstate;
valid = xstate_bv & ~XSTATE_FP_SSE;
while ( valid )
{
@@ -239,7 +239,7 @@ void expand_xsave_states(const struct vcpu *v, void *dest, unsigned int size)
*/
void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
{
- struct xsave_struct *xsave = v->arch.xsave_area;
+ struct xsave_struct *xstate = v->arch.xsave_area;
void *dest;
uint16_t comp_offsets[sizeof(xfeature_mask)*8];
u64 xstate_bv, valid;
@@ -252,7 +252,7 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
if ( !(v->arch.xcr0_accum & XSTATE_XSAVES_ONLY) )
{
- memcpy(xsave, src, size);
+ memcpy(xstate, src, size);
return;
}
@@ -260,19 +260,19 @@ void compress_xsave_states(struct vcpu *v, const void *src, unsigned int size)
* Copy legacy XSAVE area, to avoid complications with CPUID
* leaves 0 and 1 in the loop below.
*/
- memcpy(xsave, src, FXSAVE_SIZE);
+ memcpy(xstate, src, FXSAVE_SIZE);
/* Set XSTATE_BV and XCOMP_BV. */
- xsave->xsave_hdr.xstate_bv = xstate_bv;
- xsave->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
+ xstate->xsave_hdr.xstate_bv = xstate_bv;
+ xstate->xsave_hdr.xcomp_bv = v->arch.xcr0_accum | XSTATE_COMPACTION_ENABLED;
- setup_xstate_comp(comp_offsets, xsave->xsave_hdr.xcomp_bv);
+ setup_xstate_comp(comp_offsets, xstate->xsave_hdr.xcomp_bv);
/*
* Copy each region from the non-compacted offset to the
* possibly compacted offset.
*/
- dest = xsave;
+ dest = xstate;
valid = xstate_bv & ~XSTATE_FP_SSE;
while ( valid )
{
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 4/5] x86/xstate: address MISRA C:2012 Rule 5.3
2023-08-08 11:08 ` [XEN PATCH v2 4/5] x86/xstate: " Nicola Vetrini
@ 2023-08-08 14:03 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 14:03 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
xen-devel
On 08.08.2023 13:08, Nicola Vetrini wrote:
> Rename the local variables s/xsave/xstate/ to avoid clashing
> with function 'xsave' defined in 'xen/arch/x86/include/asm/xstate.h'.
s/defined/declared/ (in some areas of Misra the distinction looks to
be quite relevant, so being precise everywhere is helpful even if only
to not leave uncertainty)
> No functional changes.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h'
2023-08-08 11:08 [XEN PATCH v2 0/5] x86: address MISRA C:2012 Rule 5.3 Nicola Vetrini
` (3 preceding siblings ...)
2023-08-08 11:08 ` [XEN PATCH v2 4/5] x86/xstate: " Nicola Vetrini
@ 2023-08-08 11:08 ` Nicola Vetrini
2023-08-08 12:02 ` Nicola Vetrini
2023-08-08 14:46 ` Jan Beulich
4 siblings, 2 replies; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 11:08 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Nicola Vetrini, Jan Beulich, Andrew Cooper,
Roger Pau Monné, Wei Liu
The macros defined 'xen/include/public/arch-x86/xen-mca.h' are revised
to address the following concerns:
- needless underscore prefixes for parameter names;
- the variable 'i' in function 'mce_action' that is shadowed
by the local variable in the macro.
Therefore, the refactoring aims to resolve present shadowing
issues, which violate MISRA C:2012 Rule 5.3, and lessen the
probability of future ones with some renames.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
Changes in v2:
- Added missing parentheses.
- Clarified commit message.
---
xen/include/public/arch-x86/xen-mca.h | 38 +++++++++++++--------------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/xen/include/public/arch-x86/xen-mca.h b/xen/include/public/arch-x86/xen-mca.h
index b897536ec5..bb1b12f14f 100644
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -280,39 +280,39 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
/* Prototype:
* uint32_t x86_mcinfo_nentries(struct mc_info *mi);
*/
-#define x86_mcinfo_nentries(_mi) \
- (_mi)->mi_nentries
+#define x86_mcinfo_nentries(mi) \
+ (mi)->mi_nentries
/* Prototype:
* struct mcinfo_common *x86_mcinfo_first(struct mc_info *mi);
*/
-#define x86_mcinfo_first(_mi) \
- ((struct mcinfo_common *)(_mi)->mi_data)
+#define x86_mcinfo_first(mi) \
+ ((struct mcinfo_common *)(mi)->mi_data)
/* Prototype:
* struct mcinfo_common *x86_mcinfo_next(struct mcinfo_common *mic);
*/
-#define x86_mcinfo_next(_mic) \
- ((struct mcinfo_common *)((uint8_t *)(_mic) + (_mic)->size))
+#define x86_mcinfo_next(mic) \
+ ((struct mcinfo_common *)((uint8_t *)(mic) + (mic)->size))
/* Prototype:
- * void x86_mcinfo_lookup(void *ret, struct mc_info *mi, uint16_t type);
+ * void x86_mcinfo_lookup(void *ret, struct mc_info *mi, uint16_t mc_type);
*/
-#define x86_mcinfo_lookup(_ret, _mi, _type) \
+#define x86_mcinfo_lookup(ret, mi, mc_type) \
do { \
- uint32_t found, i; \
- struct mcinfo_common *_mic; \
+ uint32_t found_, i_; \
+ struct mcinfo_common *mic_; \
\
- found = 0; \
- (_ret) = NULL; \
- if (_mi == NULL) break; \
- _mic = x86_mcinfo_first(_mi); \
- for (i = 0; i < x86_mcinfo_nentries(_mi); i++) { \
- if (_mic->type == (_type)) { \
- found = 1; \
+ found_ = 0; \
+ (ret) = NULL; \
+ if ((mi) == NULL) break; \
+ mic_ = x86_mcinfo_first(mi); \
+ for (i_ = 0; i_ < x86_mcinfo_nentries(mi); i_++) { \
+ if (mic_->type == (mc_type)) { \
+ found_ = 1; \
break; \
} \
- _mic = x86_mcinfo_next(_mic); \
+ mic_ = x86_mcinfo_next(mic_); \
} \
- (_ret) = found ? _mic : NULL; \
+ (ret) = found_ ? mic_ : NULL; \
} while (0)
--
2.34.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h'
2023-08-08 11:08 ` [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h' Nicola Vetrini
@ 2023-08-08 12:02 ` Nicola Vetrini
2023-08-08 12:06 ` Jan Beulich
2023-08-08 14:46 ` Jan Beulich
1 sibling, 1 reply; 15+ messages in thread
From: Nicola Vetrini @ 2023-08-08 12:02 UTC (permalink / raw)
To: xen-devel
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Jan Beulich, Andrew Cooper, Roger Pau Monné,
Wei Liu
On 08/08/2023 13:08, Nicola Vetrini wrote:
> The macros defined 'xen/include/public/arch-x86/xen-mca.h' are revised
> to address the following concerns:
> - needless underscore prefixes for parameter names;
> - the variable 'i' in function 'mce_action' that is shadowed
> by the local variable in the macro.
>
> Therefore, the refactoring aims to resolve present shadowing
> issues, which violate MISRA C:2012 Rule 5.3, and lessen the
> probability of future ones with some renames.
>
> No functional change.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
> Changes in v2:
> - Added missing parentheses.
> - Clarified commit message.
> ---
Missing a
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
from the other version, since nothing changed code-wise.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h'
2023-08-08 12:02 ` Nicola Vetrini
@ 2023-08-08 12:06 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 12:06 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
xen-devel
On 08.08.2023 14:02, Nicola Vetrini wrote:
> On 08/08/2023 13:08, Nicola Vetrini wrote:
>> The macros defined 'xen/include/public/arch-x86/xen-mca.h' are revised
>> to address the following concerns:
>> - needless underscore prefixes for parameter names;
>> - the variable 'i' in function 'mce_action' that is shadowed
>> by the local variable in the macro.
>>
>> Therefore, the refactoring aims to resolve present shadowing
>> issues, which violate MISRA C:2012 Rule 5.3, and lessen the
>> probability of future ones with some renames.
>>
>> No functional change.
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> Changes in v2:
>> - Added missing parentheses.
This ...
>> - Clarified commit message.
>> ---
>
> Missing a
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> from the other version, since nothing changed code-wise.
... isn't exactly "nothing", but well.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h'
2023-08-08 11:08 ` [XEN PATCH v2 5/5] x86: refactor macros in 'xen-mca.h' Nicola Vetrini
2023-08-08 12:02 ` Nicola Vetrini
@ 2023-08-08 14:46 ` Jan Beulich
1 sibling, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-08-08 14:46 UTC (permalink / raw)
To: Nicola Vetrini
Cc: sstabellini, michal.orzel, xenia.ragiadakou, ayan.kumar.halder,
consulting, Andrew Cooper, Roger Pau Monné, Wei Liu,
xen-devel
On 08.08.2023 13:08, Nicola Vetrini wrote:
> The macros defined 'xen/include/public/arch-x86/xen-mca.h' are revised
> to address the following concerns:
> - needless underscore prefixes for parameter names;
> - the variable 'i' in function 'mce_action' that is shadowed
> by the local variable in the macro.
>
> Therefore, the refactoring aims to resolve present shadowing
> issues, which violate MISRA C:2012 Rule 5.3, and lessen the
> probability of future ones with some renames.
>
> No functional change.
>
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread