Re: [XEN PATCH v2] x86/mm: address violations of MISRA C:2012 Rules 8.2 and 8.3

[Julien Grall <julien@xxxxxxx>]

Hi Federico,

On 04/08/2023 11:29, Federico Serafini wrote:
> On 04/08/23 11:47, Andrew Cooper wrote:
> > On 04/08/2023 10:38 am, Federico Serafini wrote:
> > > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> > > index be2b10a391..e1d9b94007 100644
> > > --- a/xen/arch/x86/mm.c
> > > +++ b/xen/arch/x86/mm.c
> > > @@ -5591,7 +5591,7 @@ int __init populate_pt_range(unsigned long 
> > > virt, unsigned long nr_mfns)
> > >    *
> > >    * It is an error to call with present flags over an unpopulated 
> > > range.
> > >    */
> > > -int modify_xen_mappings(unsigned long s, unsigned long e, unsigned 
> > > int nf)
> > > +int modify_xen_mappings(unsigned long s, unsigned long e, unsigned 
> > > int flags)
> > I think both of these examples want to stay as nf (new flags).  Flags on
> > its own is ambiguous in context, and nf is a common shorthand in our
> > pagetable code.
I guess you mean x86 code? I agree that 'flags' is ambiguous but...

> > And it will make the patch rather shorter.
> >
> > ~Andrew
> The arm code has its own implementation of modify_xen_mappings()
> which uses `flags`.
> I put Stefano and Julien in CC, so that if everyone likes `nf` I can 
> propagate the change.
... I would not say I like the name 'nf'. I would prefer 'new_flags'.

Anyway, unlike arm, the x86 version of modify_xen_mappings() is quite 
large. So I would be OK to switch the Arm code to use 'nf' (only in 
modify_xen_mappings()) for the sake of avoid too much code churn.
Cheers,

--
Julien Grall