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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

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@xxxxxxxxxxx>
> > ---
> > 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.
This one is defined as

return e820_change_range_type(e820, s, e, E820_RAM, E820_RESERVED);

so I'm not certain that the parameter can be dropped from that function, 
because the cascade effect
would eliminate the need to have a 'boot_e820' in 'xen/arch/x86/setup.c' 
afaict and since the comment says
/* A temporary copy of the e820 map that we can mess with during 
bootstrap. */
static struct e820map __initdata boot_e820;

I'm not sure it's a good idea to alter this call chain.

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