Re: [XEN PATCH 1/6] x86: rename variable 'e820' to address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

On 07/08/2023 10:09, Jan Beulich wrote:
> On 04.08.2023 17:27, Nicola Vetrini wrote:
>> The variable declared in the header file 
>> 'xen/arch/x86/include/asm/e820.h'
>> is shadowed by many function parameters, so it is renamed to avoid 
>> these
>> violations.
>> 
>> No functional changes.
>> 
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>> This patch is similar to other renames done on previous patches, and 
>> the
>> preferred strategy there was to rename the global variable. This one
>> has more occurrences that are spread in various files, but
>> the general pattern is the same.
> 
> Still I think it would be better done the other way around, and perhaps 
> in
> more than a single patch. It looks like "many == 3", i.e.
> - e820_add_range(), which is only ever called with "e820" as its 
> argument,
>   and hence the parameter could be dropped,
> - e820_change_range_type(), which is in the same situation, and
> - reserve_e820_ram(), which wants its parameter renamed.
> Alternatively, if we really were to change the name of the global, we'd
> want to take a more complete approach: Right now we have e820_raw[],
> boot_e820[], and e820[]. We'd want them to follow a uniform naming 
> scheme
> then (e820_ first or _e820 last), with the other part of the name 
> suitably
> describing the purpose (which "map" doesn't do).
> 
> Jan

Besides the one you listed, there are these other occurrences:
- xen/arch/x86/mm.c:4678 in 'arch_memory_op' as local variable 'struct 
e820entry'
- xen/arch/x86/include/asm/guest/hypervisor.h:55 in 
'hypervisor_e820_fixup'
- xen/arch/x86/include/asm/pv/shim.h:88 in 'pv_shim_fixup'
- xen/arch/x86/setup.c:689 in 'kexec_reserve_area'

We can take the first approach you suggested (which was my original 
attempt, but then upon feedback on other
patches I reworked this patch before submitting). My doubt about it was 
that it would introduce a naming
inconsistency with other e820-related objects/types. Anyway, if e820_map 
is not a good name, could e820_arr be it?

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)