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@xxxxxxxxxx>
> > ---
> > 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
the function, indicating that it only works within this function, just
like what this patch did. Alternatively, we can move this macro
definition to the beginning of the file, so that it can be used
throughout the module as shown below.
What do you think of these modifications? Will they make it more
convenient for others who read this code?
Best Regards,
Enze
--- a/arch/mips/mm/tlbex.c
+++ b/arch/mips/mm/tlbex.c
@@ -37,6 +37,9 @@
#include <asm/setup.h>
#include <asm/tlbex.h>
+#define pr_define(fmt, ...) \
+ pr_debug("#define " fmt, ##__VA_ARGS__)
+
static int mips_xpa_disabled;
static int __init xpa_disable(char *s)
@@ -231,9 +234,6 @@ static void uasm_bgezl_label(struct uasm_label **l,
u32 **p, int instance)
*/
static void output_pgtable_bits_defines(void)
{
-#define pr_define(fmt, ...) \
- pr_debug("#define " fmt, ##__VA_ARGS__)
-
pr_debug("#include <asm/asm.h>\n");
pr_debug("#include <asm/regdef.h>\n");
pr_debug("\n");
