Re: [XEN PATCH v3] xen/spinlock: mechanically rename parameter name 'debug'

[Julien Grall <julien@xxxxxxx>]

Hi,

On 27/07/2023 20:35, Stefano Stabellini wrote:
> On Thu, 27 Jul 2023, Jan Beulich wrote:
> > On 26.07.2023 23:49, Stefano Stabellini wrote:
> > > On Wed, 26 Jul 2023, Jan Beulich wrote:
> > > > On 26.07.2023 08:42, Nicola Vetrini wrote:
> > > > > On 26/07/23 08:34, Jan Beulich wrote:
> > > > > > On 25.07.2023 22:45, 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"
> > > > > > >
> > > > > > > To avoid any confusion resulting from the parameter 'debug'
> > > > > > > hiding the homonymous function declared at
> > > > > > > 'xen/arch/x86/include/asm/processor.h:428'
> > > > > > > the rename of parameters s/debug/lkdbg/ is performed.
> > > > > > >
> > > > > > > Signed-off-by: Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>
> > > > > > > ---
> > > > > > > Changes in v2:
> > > > > > > - s/dbg/lkdbg/
> > > > > > > Changes in v3:
> > > > > > > - Added missing renames for consistency
> > > > > > Hmm, you asked whether to send v3, but then you didn't wait for an
> > > > > > answer. So to repeat what I said there: I'd prefer if we could first
> > > > > > settle whether to rename the conflicting x86 symbol.
> > > > > Stefano replied asking for a v3 [1] before I had a chance to read your
> > > > > message this morning.
> > > > Right, sorry, I spotted his reply only after seeing the v3.
> > > For what is worth I prefer the current implementation compared to
> > > renaming debug()
> > I don't. My replacement name suggestions were only "just in case"; I
> > don't really like them.
> Understood :-)
>
> How would you like to proceed?
>
> 1. we commit this patch as is
> 2. we wait for a third opinion from another maintainer
> 3. we find a new name for the variable
> 4. we change debug() instead
IMO, the name debug() is quite generic and it is not obvious that the 
function is a trap handler. So I think renaming debug() is the right way 
to go.
Cheers,

--
Julien Grall