Re: [XEN PATCH 1/4] x86: mechanically rename to address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 27/07/23 17:00, Andrew Cooper wrote:
> On 27/07/2023 3:50 pm, Jan Beulich wrote:
> > On 27.07.2023 12:47, Nicola Vetrini wrote:
> > > Rule 5.3 has the following headline:
> > > "An identifier declared in an inner scope shall not hide an
> > > identifier declared in an outer scope"
> > >
> > > The renames done by this patch avoid shadowing from happening.
> > > They are as follows:
> > > - s/str/s/ in 'lapic_disable'
> > > - s/str/level/ in '(apic|mce)_set_verbosity'
> > > - s/str/state_str/ in 'mwait_idle_probe'
> > > - s/str/memmap_name/ in 'init_e820'
> > I'm sorry to say that, but I'm not willing to go and figure out where
> > that "str" is that there's supposedly a collision with. Please can you
> > state such right here, ...
> >
> > > - s/i/j/ in 'mce_action' (the shadowing here is due to macro
> > >    'x86_mcinfo_lookup' that defines 'i' as a loop counter)
> > ... much like you do in this case?
> In fairness to Nicola, that was given.
>
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > ---
> > Function 'str' in 'xen/arch/x86/include/asm/desc.h'
> > causes the shadowing.
> which is the wrapper for the STR instruction.
>
> It's used in a single assertion, and I'd be happy getting rid of it
> entirely.  Alternatively, it could be renamed to read_tr() (or
> read_tr_sel() ?) if we want to keep the assertion.
>
> We're not renaming every other use of 'str' to mean string just for this...
>
> ~Andrew
Seems reasonable to remove it, though there aren't that many instances 
of shadowing on 'str'.
--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)