Re: [XEN PATCH v2 4/4] x86/setup: address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 07/08/2023 15:05, Jan Beulich wrote:
> On 04.08.2023 10:03, Nicola Vetrini wrote:
> > The parameters renamed in the function declaration caused shadowing
> > with the homonymous variable in 'xen/common/efi/boot.c'. Renaming
> > them also addresses Rule 8.3:
> > "All declarations of an object or function shall use the same names
> > and type qualifiers".
> Can you explain to me how shadowing can happen in a declaration? I
> would focus on 8.3 here, and only mention the other name collision.
There's "static struct file __initdata kernel;" in 
xen/common/efi/boot.c, which
is visible when the function is declared. Since renaming these parameter 
names would
have been addressed by Federico for R8.3 anyway, my intention was to 
address them both.
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -1577,8 +1577,7 @@ void __init noreturn __start_xen(unsigned long 
> > mbi_p)
> >          s = map_s;
> >          if ( s < map_e )
> >          {
> > -            uint64_t mask = (1UL << L2_PAGETABLE_SHIFT) - 1;
> > -
> > +            mask = (1UL << L2_PAGETABLE_SHIFT) - 1;
> >              map_s = (s + mask) & ~mask;
> >              map_e &= ~mask;
> >              init_boot_pages(map_s, map_e);
> Re-using the outer scope variable is a little risky, don't you agree?
> It just so happens that below here there's no further use requiring
> the earlier value (PAGE_SIZE - 1). This isn't to say I'm opposed, but
> it certainly needs evaluating with this in mind (mentioning of which
> in the description would have demonstrated that you did consider this
> aspect).
>
> Jan
I guess I should have mentioned it

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