Re: [PATCH] MIPS: tlbex: undefine pr_define macro when appropriate

[Thomas Bogendoerfer <tsbogend@alpha.franken.de>]

On Sat, Apr 01, 2023 at 05:14:28PM +0800, Enze Li wrote:
> On Sat, 2023-04-01 at 09:17 +0200, Thomas Bogendoerfer wrote:
> > On Sat, Apr 01, 2023 at 02:10:25PM +0800, Enze Li wrote:
> > > The pr_define macro is used only within the
> > > output_pgtable_bits_defines
> > > function, and it hasn't been used anywhere else so far.  Therefore,
> > > it
> > > should be undefined when appropriate.
> > > 
> > > Signed-off-by: Enze Li <lienze@kylinos.cn>
> > > ---
> > >  arch/mips/mm/tlbex.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> > > index 80e05ee98d62..510a7e316eb6 100644
> > > --- a/arch/mips/mm/tlbex.c
> > > +++ b/arch/mips/mm/tlbex.c
> > > @@ -255,6 +255,8 @@ static void output_pgtable_bits_defines(void)
> > >         pr_define("_PAGE_DIRTY_SHIFT %d\n", _PAGE_DIRTY_SHIFT);
> > >         pr_define("_PFN_SHIFT %d\n", _PFN_SHIFT);
> > >         pr_debug("\n");
> > > +
> > > +#undef pr_define
> > 
> > I'm probably missing something... what problem are you fixing here ?
> > 
> > Thomas.
> > 
> 
> Hi Thomas,
> 
> Thanks for your review.  🙂
> 
> I'm trying to make the code more readable.  When I first looked at this
> function, I was wondering why the pr_define macro was defined here.  It
> looks more like a temporary definition, and I was also curious if it
> might be used elsewhere.  After searching, I couldn't find any other
> instances of it being used, which left me feeling confused.
> 
> To enhance readability, it would be good to undefine it at the end of

why is it more readable ?

> What do you think of these modifications?  Will they make it more
> convenient for others who read this code?

for me the missing undef doesn't hurt as there is no rule that local
#defines need to be undefined. And less lines distracting me is a plus

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]