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

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

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)