Re: [XEN PATCH 2/4] x86/mtrr: address MISRA C:2012 Rule 5.3

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 03/08/2023 03:51, Stefano Stabellini wrote:
> On Wed, 2 Aug 2023, Nicola Vetrini wrote:
> > Rename variables to avoid shadowing and thus address
> > MISRA C:2012 Rule 5.3:
> > "An identifier declared in an inner scope shall not hide an
> > identifier declared in an outer scope"
> >
> > No functional changes.
> >
> > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> This one clashes with xen/arch/x86/include/asm/mtrr.h:struct mtrr_state
> right?
Yes, indeed. You can also check this when in doubt at
https://saas.eclairit.com, but I do agree that mentioning it directly in 
the commit
would make review easier.

> > ---
> >  xen/arch/x86/hvm/mtrr.c | 32 ++++++++++++++++----------------
> >  1 file changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/mtrr.c b/xen/arch/x86/hvm/mtrr.c
> > index 29f3fb1607..d504d1e43b 100644
> > --- a/xen/arch/x86/hvm/mtrr.c
> > +++ b/xen/arch/x86/hvm/mtrr.c
> > @@ -687,13 +687,13 @@ int hvm_set_mem_pinned_cacheattr(struct domain 
> > *d, uint64_t gfn_start,
> >  static int cf_check hvm_save_mtrr_msr(struct vcpu *v, 
> > hvm_domain_context_t *h)
> >  {
> > -    const struct mtrr_state *mtrr_state = &v->arch.hvm.mtrr;
> > +    const struct mtrr_state *mtrr = &v->arch.hvm.mtrr;
> I can see in hvm_vcpu_cacheattr_init, mtrr_get_type and other places
> that we use:
>
>   const struct mtrr_state *m
>
> maybe that's better? I'll let Jan comment.
>
> Anyway, the patch looks correct to me.
I though about it, and it seemed too generic, so in the end I decided 
for mtrr, but
as always with names, I'm open to alternative suggestions.

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