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

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 04/08/2023 23:19, Stefano Stabellini wrote:
> On Fri, 4 Aug 2023, 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.
> > ---
> >  xen/arch/x86/dom0_build.c                | 10 ++--
> >  xen/arch/x86/e820.c                      | 66 
> > ++++++++++++------------
> >  xen/arch/x86/guest/xen/xen.c             |  4 +-
> >  xen/arch/x86/hvm/dom0_build.c            |  6 +--
> >  xen/arch/x86/include/asm/e820.h          |  2 +-
> >  xen/arch/x86/mm.c                        | 49 +++++++++---------
> >  xen/arch/x86/numa.c                      |  8 +--
> >  xen/arch/x86/setup.c                     | 22 ++++----
> >  xen/arch/x86/srat.c                      |  6 +--
> >  xen/arch/x86/x86_64/mmconf-fam10h.c      |  2 +-
> >  xen/drivers/passthrough/amd/iommu_acpi.c |  2 +-
> There are missing changes to xen/arch/x86/tboot.c
Thanks, I'll replace them.

> There a few stray changes below.
>
> Everything else is correct.
>
>
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 80ae973d64..9c6003e374 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -1163,7 +1163,7 @@ void __init noreturn __start_xen(unsigned long 
> > mbi_p)
> >      }
> >      else if ( efi_enabled(EFI_BOOT) )
> >          memmap_type = "EFI";
> > -    else if ( (e820_raw.nr_map =
> > +    else if ( (e820_raw.nr_map =
> stray change
> > @@ -1631,7 +1631,7 @@ void __init noreturn __start_xen(unsigned long 
> > mbi_p)
> >          unsigned long e = min(s + PFN_UP(kexec_crash_area.size),
> >                                PFN_UP(__pa(HYPERVISOR_VIRT_END - 1)));
> >
> > -        if ( e > s )
> > +        if ( e > s )
> stray change
> > @@ -1771,7 +1771,7 @@ void __init noreturn __start_xen(unsigned long 
> > mbi_p)
> >      open_softirq(NEW_TLBFLUSH_CLOCK_PERIOD_SOFTIRQ, 
> > new_tlbflush_clock_period);
> > -    if ( opt_watchdog )
> > +    if ( opt_watchdog )
> >          nmi_watchdog = NMI_LOCAL_APIC;
> stray change
>
>
> >      find_smp_config();
> > @@ -1983,7 +1983,7 @@ void __init noreturn __start_xen(unsigned long 
> > mbi_p)
> >      do_initcalls();
> >
> > -    if ( opt_watchdog )
> > +    if ( opt_watchdog )
> >          watchdog_setup();
> stray change
I looked at those, and there were trailing blanks on those lines, which 
apparently
got trimmed when renaming the variable. I think these changes are ok
(though maybe they should be mentioned in the commit message).

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