Re: [PATCH] asm-generic: replace ________addr with __UNIQUE_ID(addr)

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 22 Mar 2026 14:40:32 +0000 David Laight <david.laight.linux@gmail.com> wrote:

> > -#define __set_fixmap_offset(idx, phys, flags)				\
> > -({									\
> > -	unsigned long ________addr;					\
> > -	__set_fixmap(idx, phys, flags);					\
> > -	________addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1));	\
> > -	________addr;							\
> > +#define ___set_fixmap_offset(idx, phys, flags, uniq)            \
> > +({                                                              \
> > +	unsigned long uniq;                                         \
> > +	__set_fixmap(idx, phys, flags);                             \
> > +	uniq = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1));       \
> > +	uniq;                                                       \
> 
> You don't need a variable to hold the result at all.
> 
> The real problem with this define is that both idx and phys are
> expanded twice.

The real problem with this define is that it's a define.  Why oh why do
we keep doing this to ourselves?


What's wrong with the below?

/* Return a pointer with offset calculated */
static inline unsigned long
__set_fixmap_offset(enum fixed_addresses idx, phys_addr_t phys, pgprot_t flags)
{
	__set_fixmap(idx, phys, flags);
	return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1));
}

static inline unsigned long
set_fixmap_offset(enum fixed_addresses idx, phys_addr_t phys)
{
	return __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL);
}

/*
 * Some hardware wants to get fixmapped without caching.
 */
static inline void
set_fixmap_nocache(enum fixed_addresses idx, phys_addr_t phys)
{
	__set_fixmap(idx, phys, FIXMAP_PAGE_NOCACHE);
}

static inline void
set_fixmap_offset_nocache(enum fixed_addresses idx, phys_addr_t phys)
{
	__set_fixmap_offset(idx, phys, FIXMAP_PAGE_NOCACHE);
}


I'll toss the below into mm.git, shall send it to Arnd if nothing blows
up.


From: Andrew Morton <akpm@linux-foundation.org>
Subject: include/asm-generic/fixmap.h: reimplement nasty macros in C
Date: Sat Apr 25 01:42:28 PM PDT 2026

Min-Hsun Chang reports[1] "the macro __set_fixmap_offset() uses a
hardcoded identifier ________addr, which can lead to variable name
shadowing if a caller happens to use the same name in its scope."

As is usual with macro messes, the answer is to reimplement everything in
C.

Reported-by: Min-Hsun Chang <chmh0624@gmail.com>
Closes: https://lore.kernel.org/20260307092119.20733-1-chmh0624@gmail.com [1]
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Mark Salter <msalter@redhat.com>
Cc: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/asm-generic/fixmap.h |   34 ++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

--- a/include/asm-generic/fixmap.h~a
+++ a/include/asm-generic/fixmap.h
@@ -71,25 +71,33 @@ static inline unsigned long virt_to_fix(
 #endif
 
 /* Return a pointer with offset calculated */
-#define __set_fixmap_offset(idx, phys, flags)				\
-({									\
-	unsigned long ________addr;					\
-	__set_fixmap(idx, phys, flags);					\
-	________addr = fix_to_virt(idx) + ((phys) & (PAGE_SIZE - 1));	\
-	________addr;							\
-})
+static inline unsigned long
+__set_fixmap_offset(enum fixed_addresses idx, phys_addr_t phys, pgprot_t flags)
+{
+	__set_fixmap(idx, phys, flags);
+	return fix_to_virt(idx) + (phys & (PAGE_SIZE - 1));
+}
 
-#define set_fixmap_offset(idx, phys) \
-	__set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL)
+static inline unsigned long
+set_fixmap_offset(enum fixed_addresses idx, phys_addr_t phys)
+{
+	return __set_fixmap_offset(idx, phys, FIXMAP_PAGE_NORMAL);
+}
 
 /*
  * Some hardware wants to get fixmapped without caching.
  */
-#define set_fixmap_nocache(idx, phys) \
-	__set_fixmap(idx, phys, FIXMAP_PAGE_NOCACHE)
+static inline void
+set_fixmap_nocache(enum fixed_addresses idx, phys_addr_t phys)
+{
+	__set_fixmap(idx, phys, FIXMAP_PAGE_NOCACHE);
+}
 
-#define set_fixmap_offset_nocache(idx, phys) \
-	__set_fixmap_offset(idx, phys, FIXMAP_PAGE_NOCACHE)
+static inline void
+set_fixmap_offset_nocache(enum fixed_addresses idx, phys_addr_t phys)
+{
+	__set_fixmap_offset(idx, phys, FIXMAP_PAGE_NOCACHE);
+}
 
 /*
  * Some fixmaps are for IO
_