[PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Qian-Yu Lin]

The macro trace_printk() uses a hardcoded identifier _______STR
within a statement expression, which can lead to variable name
shadowing if a caller happens to use the same name in its scope.

Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
 ("min/max: remove sparse warnings when they're nested"), replace the
 hardcoded identifier with __UNIQUE_ID(STR).

Since __UNIQUE_ID() must be expanded once to remain consistent across
declaration and sizeof() within the statement expression, introduce a
nested helper macro ___trace_printk.

Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
 include/linux/trace_printk.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..060eccb40838 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -2,6 +2,7 @@
 #ifndef _LINUX_TRACE_PRINTK_H
 #define _LINUX_TRACE_PRINTK_H
 
+#include <linux/compiler.h>
 #include <linux/compiler_attributes.h>
 #include <linux/instruction_pointer.h>
 #include <linux/stddef.h>
@@ -84,15 +85,18 @@ do {									\
  * let gcc optimize the rest.
  */
 
-#define trace_printk(fmt, ...)				\
+#define ___trace_printk(fmt, str, ...)				\
 do {							\
-	char _______STR[] = __stringify((__VA_ARGS__));	\
-	if (sizeof(_______STR) > 3)			\
+	char str[] = __stringify((__VA_ARGS__));	\
+	if (sizeof(str) > 3)			\
 		do_trace_printk(fmt, ##__VA_ARGS__);	\
 	else						\
 		trace_puts(fmt);			\
 } while (0)
 
+#define trace_printk(fmt, ...) \
+	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
+
 #define do_trace_printk(fmt, args...)					\
 do {									\
 	static const char *trace_printk_fmt __used			\
-- 
2.25.1

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Steven Rostedt]

On Thu, 30 Apr 2026 00:57:07 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:

> The macro trace_printk() uses a hardcoded identifier _______STR
> within a statement expression, which can lead to variable name
> shadowing if a caller happens to use the same name in its scope.

Has this ever been a problem?

> 
> Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
>  ("min/max: remove sparse warnings when they're nested"), replace the
>  hardcoded identifier with __UNIQUE_ID(STR).
> 
> Since __UNIQUE_ID() must be expanded once to remain consistent across
> declaration and sizeof() within the statement expression, introduce a
> nested helper macro ___trace_printk.

Hmm, so we are replacing one name with underscores with another name
with underscores?

> 
> Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> ---
>  include/linux/trace_printk.h | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> index 2670ec7f4262..060eccb40838 100644
> --- a/include/linux/trace_printk.h
> +++ b/include/linux/trace_printk.h
> @@ -2,6 +2,7 @@
>  #ifndef _LINUX_TRACE_PRINTK_H
>  #define _LINUX_TRACE_PRINTK_H
>  
> +#include <linux/compiler.h>

People are already saying that trace_printk.h slows down the compile.
Does this add any overhead to the compile?

-- Steve


>  #include <linux/compiler_attributes.h>
>  #include <linux/instruction_pointer.h>
>  #include <linux/stddef.h>
> @@ -84,15 +85,18 @@ do {									\
>   * let gcc optimize the rest.
>   */
>  
> -#define trace_printk(fmt, ...)				\
> +#define ___trace_printk(fmt, str, ...)				\
>  do {							\
> -	char _______STR[] = __stringify((__VA_ARGS__));	\
> -	if (sizeof(_______STR) > 3)			\
> +	char str[] = __stringify((__VA_ARGS__));	\
> +	if (sizeof(str) > 3)			\
>  		do_trace_printk(fmt, ##__VA_ARGS__);	\
>  	else						\
>  		trace_puts(fmt);			\
>  } while (0)
>  
> +#define trace_printk(fmt, ...) \
> +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> +
>  #define do_trace_printk(fmt, args...)					\
>  do {									\
>  	static const char *trace_printk_fmt __used			\

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[David Laight]

On Wed, 29 Apr 2026 13:42:26 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Thu, 30 Apr 2026 00:57:07 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> 
> > The macro trace_printk() uses a hardcoded identifier _______STR
> > within a statement expression, which can lead to variable name
> > shadowing if a caller happens to use the same name in its scope.  
> 
> Has this ever been a problem?
> 
> > 
> > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> >  ("min/max: remove sparse warnings when they're nested"), replace the
> >  hardcoded identifier with __UNIQUE_ID(STR).

min and max do get nested - so it is important that the 'local'
variables have different names - otherwise you can get invalid expansions.

No one is going to have: trace_printk(fmt1, trace_printk(ftm2, ...), ...)
it just doesn't make sense.

There is a slight problem the ____________________________STR might
be used by an entirely different #define.
Just using _trace_printk_str[] would make this pretty unlikely.
It would also have the advantage of making the .i file a bit easier
to read, all the UNIQUE names in min/max output make it really hard
to see what the output actually means.

> > 
> > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > declaration and sizeof() within the statement expression, introduce a
> > nested helper macro ___trace_printk.  
> 
> Hmm, so we are replacing one name with underscores with another name
> with underscores?
> 
> > 
> > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > ---
> >  include/linux/trace_printk.h | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > index 2670ec7f4262..060eccb40838 100644
> > --- a/include/linux/trace_printk.h
> > +++ b/include/linux/trace_printk.h
> > @@ -2,6 +2,7 @@
> >  #ifndef _LINUX_TRACE_PRINTK_H
> >  #define _LINUX_TRACE_PRINTK_H
> >  
> > +#include <linux/compiler.h>  
> 
> People are already saying that trace_printk.h slows down the compile.
> Does this add any overhead to the compile?

A little - nothing is free.

	David

> 
> -- Steve
> 
> 
> >  #include <linux/compiler_attributes.h>
> >  #include <linux/instruction_pointer.h>
> >  #include <linux/stddef.h>
> > @@ -84,15 +85,18 @@ do {									\
> >   * let gcc optimize the rest.
> >   */
> >  
> > -#define trace_printk(fmt, ...)				\
> > +#define ___trace_printk(fmt, str, ...)				\
> >  do {							\
> > -	char _______STR[] = __stringify((__VA_ARGS__));	\
> > -	if (sizeof(_______STR) > 3)			\
> > +	char str[] = __stringify((__VA_ARGS__));	\
> > +	if (sizeof(str) > 3)			\
> >  		do_trace_printk(fmt, ##__VA_ARGS__);	\
> >  	else						\
> >  		trace_puts(fmt);			\
> >  } while (0)
> >  
> > +#define trace_printk(fmt, ...) \
> > +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > +
> >  #define do_trace_printk(fmt, args...)					\
> >  do {									\
> >  	static const char *trace_printk_fmt __used			\  
> 
> 

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Qian-Yu Lin]

On Wed, Apr 29, 2026 at 10:47:28PM +0100, David Laight wrote:
> On Wed, 29 Apr 2026 13:42:26 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > On Thu, 30 Apr 2026 00:57:07 +0800
> > Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> > 
> > > The macro trace_printk() uses a hardcoded identifier _______STR
> > > within a statement expression, which can lead to variable name
> > > shadowing if a caller happens to use the same name in its scope.  
> > 
> > Has this ever been a problem?
> > 
> > > 
> > > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> > >  ("min/max: remove sparse warnings when they're nested"), replace the
> > >  hardcoded identifier with __UNIQUE_ID(STR).
> 
> min and max do get nested - so it is important that the 'local'
> variables have different names - otherwise you can get invalid expansions.
> 
> No one is going to have: trace_printk(fmt1, trace_printk(ftm2, ...), ...)
> it just doesn't make sense.
> 
> There is a slight problem the ____________________________STR might
> be used by an entirely different #define.
> Just using _trace_printk_str[] would make this pretty unlikely.
> It would also have the advantage of making the .i file a bit easier
> to read, all the UNIQUE names in min/max output make it really hard
> to see what the output actually means.
> 

Thank you for the suggestion. However, _trace_printk_str still carries
a theoretical shadowing risk if a caller happens to use the same name.

> > > 
> > > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > > declaration and sizeof() within the statement expression, introduce a
> > > nested helper macro ___trace_printk.  
> > 
> > Hmm, so we are replacing one name with underscores with another name
> > with underscores?
> > 
> > > 
> > > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > > ---
> > >  include/linux/trace_printk.h | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > > index 2670ec7f4262..060eccb40838 100644
> > > --- a/include/linux/trace_printk.h
> > > +++ b/include/linux/trace_printk.h
> > > @@ -2,6 +2,7 @@
> > >  #ifndef _LINUX_TRACE_PRINTK_H
> > >  #define _LINUX_TRACE_PRINTK_H
> > >  
> > > +#include <linux/compiler.h>  
> > 
> > People are already saying that trace_printk.h slows down the compile.
> > Does this add any overhead to the compile?
> 
> A little - nothing is free.
> 
> 	David
> 

Indeed. I measured compile time of kernel/trace/ring_buffer_benchmark.o
after make clean on an x86_64 machine running Ubuntu 24.04 LTS:

  - Original _______STR:                 49.8s
  - v1 with __UNIQUE_ID (compiler.h):    53.5s
  - compound literal (no extra include): 33.2s

A compound literal eliminates the local variable entirely, removing
the shadowing risk without any extra include or compile overhead:

#define trace_printk(fmt, ...)                          \
do {                                                    \
    if (sizeof((char[])                             \
        {__stringify((__VA_ARGS__))}) > 3)      \
        do_trace_printk(fmt, ##__VA_ARGS__);    \
    else                                            \
        trace_puts(fmt);                        \
} while (0)

Qian-Yu

> > 
> > -- Steve
> > 
> > 
> > >  #include <linux/compiler_attributes.h>
> > >  #include <linux/instruction_pointer.h>
> > >  #include <linux/stddef.h>
> > > @@ -84,15 +85,18 @@ do {									\
> > >   * let gcc optimize the rest.
> > >   */
> > >  
> > > -#define trace_printk(fmt, ...)				\
> > > +#define ___trace_printk(fmt, str, ...)				\
> > >  do {							\
> > > -	char _______STR[] = __stringify((__VA_ARGS__));	\
> > > -	if (sizeof(_______STR) > 3)			\
> > > +	char str[] = __stringify((__VA_ARGS__));	\
> > > +	if (sizeof(str) > 3)			\
> > >  		do_trace_printk(fmt, ##__VA_ARGS__);	\
> > >  	else						\
> > >  		trace_puts(fmt);			\
> > >  } while (0)
> > >  
> > > +#define trace_printk(fmt, ...) \
> > > +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > +
> > >  #define do_trace_printk(fmt, args...)					\
> > >  do {									\
> > >  	static const char *trace_printk_fmt __used			\  
> > 
> > 
> 

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Qian-Yu Lin]

On Wed, Apr 29, 2026 at 01:42:26PM -0400, Steven Rostedt wrote:
> On Thu, 30 Apr 2026 00:57:07 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> 
> > The macro trace_printk() uses a hardcoded identifier _______STR
> > within a statement expression, which can lead to variable name
> > shadowing if a caller happens to use the same name in its scope.
> 
> Has this ever been a problem?
> 

No, this has not been a practical problem. However, the long-underscore
name ___STR is an indication of a potential shadowing risk.

> > 
> > Following the pattern in commit 24ba53017e18 ("rcu: Replace ________p1
> > and _________p1 with __UNIQUE_ID(rcu)") and commit 589a9785ee3a
> >  ("min/max: remove sparse warnings when they're nested"), replace the
> >  hardcoded identifier with __UNIQUE_ID(STR).
> > 
> > Since __UNIQUE_ID() must be expanded once to remain consistent across
> > declaration and sizeof() within the statement expression, introduce a
> > nested helper macro ___trace_printk.
> 
> Hmm, so we are replacing one name with underscores with another name
> with underscores?
> 
> > 
> > Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
> > ---
> >  include/linux/trace_printk.h | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
> > index 2670ec7f4262..060eccb40838 100644
> > --- a/include/linux/trace_printk.h
> > +++ b/include/linux/trace_printk.h
> > @@ -2,6 +2,7 @@
> >  #ifndef _LINUX_TRACE_PRINTK_H
> >  #define _LINUX_TRACE_PRINTK_H
> >  
> > +#include <linux/compiler.h>
> 
> People are already saying that trace_printk.h slows down the compile.
> Does this add any overhead to the compile?
> 
> -- Steve

Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
after make clean on an x86_64 machine running Ubuntu 24.04 LTS:

  - Original _______STR:                 49.8s
  - v1 with __UNIQUE_ID (compiler.h):    53.5s
  - compound literal (no extra include): 33.2s

I propose using a compound literal in v2, which eliminates the local
variable entirely and requires no extra include:

#define trace_printk(fmt, ...)                          \
do {                                                    \
    if (sizeof((char[])                             \
        {__stringify((__VA_ARGS__))}) > 3)      \
        do_trace_printk(fmt, ##__VA_ARGS__);    \
    else                                            \
        trace_puts(fmt);                        \
} while (0)

This fully eliminates the shadowing risk without any compile overhead.

Qian-Yu

> 
> 
> >  #include <linux/compiler_attributes.h>
> >  #include <linux/instruction_pointer.h>
> >  #include <linux/stddef.h>
> > @@ -84,15 +85,18 @@ do {									\
> >   * let gcc optimize the rest.
> >   */
> >  
> > -#define trace_printk(fmt, ...)				\
> > +#define ___trace_printk(fmt, str, ...)				\
> >  do {							\
> > -	char _______STR[] = __stringify((__VA_ARGS__));	\
> > -	if (sizeof(_______STR) > 3)			\
> > +	char str[] = __stringify((__VA_ARGS__));	\
> > +	if (sizeof(str) > 3)			\
> >  		do_trace_printk(fmt, ##__VA_ARGS__);	\
> >  	else						\
> >  		trace_puts(fmt);			\
> >  } while (0)
> >  
> > +#define trace_printk(fmt, ...) \
> > +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > +
> >  #define do_trace_printk(fmt, args...)					\
> >  do {									\
> >  	static const char *trace_printk_fmt __used			\
> 

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Steven Rostedt]

On Fri, 1 May 2026 22:40:17 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:

> I propose using a compound literal in v2, which eliminates the local
> variable entirely and requires no extra include:
> 
> #define trace_printk(fmt, ...)                          \
> do {                                                    \
>     if (sizeof((char[])                             \
>         {__stringify((__VA_ARGS__))}) > 3)      \
>         do_trace_printk(fmt, ##__VA_ARGS__);    \
>     else                                            \
>         trace_puts(fmt);                        \
> } while (0)
> 
> This fully eliminates the shadowing risk without any compile overhead.

Have you tested to make sure a string with no arguments still produces the
trace_puts() and one that has arguments calls do_trace_printk()?

I'm fine with that one if it still works.

-- Steve

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Qian-Yu Lin]

On Fri, May 01, 2026 at 11:19:39AM -0400, Steven Rostedt wrote:
> On Fri, 1 May 2026 22:40:17 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> 
> > I propose using a compound literal in v2, which eliminates the local
> > variable entirely and requires no extra include:
> > 
> > #define trace_printk(fmt, ...)                          \
> > do {                                                    \
> >     if (sizeof((char[])                             \
> >         {__stringify((__VA_ARGS__))}) > 3)      \
> >         do_trace_printk(fmt, ##__VA_ARGS__);    \
> >     else                                            \
> >         trace_puts(fmt);                        \
> > } while (0)
> > 
> > This fully eliminates the shadowing risk without any compile overhead.
> 
> Have you tested to make sure a string with no arguments still produces the
> trace_puts() and one that has arguments calls do_trace_printk()?
> 
> I'm fine with that one if it still works.
> 
> -- Steve

Yes, I verified it with the preprocessor output. I created a minimal
test file:

// kernel/trace/test_trace_printk.c
#include <linux/trace_printk.h>

void test(void) {
    trace_printk("no args\n");
    trace_printk("with arg %d\n", 42);
}

Then ran make kernel/trace/test_trace_printk.i.

The no-args case has sizeof((char[]){"()"}) which is 3, so 3 > 3 is
false and it falls through to trace_bputs/trace_puts.

The args case has sizeof((char[]){"(42)"}) which is 5, so 5 > 3 is
true and it goes to trace_bprintk/trace_printk with the argument
42 correctly passed through.

Qian-Yu

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Steven Rostedt]

On Sat, 2 May 2026 00:17:50 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:

> Yes, I verified it with the preprocessor output. I created a minimal
> test file:
> 
> // kernel/trace/test_trace_printk.c
> #include <linux/trace_printk.h>
> 
> void test(void) {
>     trace_printk("no args\n");
>     trace_printk("with arg %d\n", 42);
> }
> 
> Then ran make kernel/trace/test_trace_printk.i.
> 
> The no-args case has sizeof((char[]){"()"}) which is 3, so 3 > 3 is
> false and it falls through to trace_bputs/trace_puts.
> 
> The args case has sizeof((char[]){"(42)"}) which is 5, so 5 > 3 is
> true and it goes to trace_bprintk/trace_printk with the argument
> 42 correctly passed through.

Thanks. When I test, I do the same but just compile down to an object file
and run objdump and look at which functions were called.

-- Steve

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[David Laight]

On Fri, 1 May 2026 22:40:17 +0800
Qian-Yu Lin <tiffany019230@gmail.com> wrote:

...
> Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
> after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
> 
>   - Original _______STR:                 49.8s
>   - v1 with __UNIQUE_ID (compiler.h):    53.5s
>   - compound literal (no extra include): 33.2s

That difference looks far to big to me.
And the times are far too large to be measuring the actual compile time.

> 
> I propose using a compound literal in v2, which eliminates the local
> variable entirely and requires no extra include:
> 
> #define trace_printk(fmt, ...)                          \
> do {                                                    \
>     if (sizeof((char[])                             \
>         {__stringify((__VA_ARGS__))}) > 3)      \
>         do_trace_printk(fmt, ##__VA_ARGS__);    \

There has to be a better way to align that code.
Although you should be able to use:
	if (sizeof __stringify((__VA_ARGS__)) > 3)
(I've omitted one set of parenthesis for clarity)

You could change __stringify() to work with __VA_ARGS__ the you don't need
the extra (); this works fine:
#define _x(...) #__VA_ARGS__
#define x(...) _x(__VA_ARGS__)
#define z abcd
int a = sizeof x(z, v); /* 8 */
See: https://godbolt.org/z/zo4h4nr9b

-- David

>     else                                            \
>         trace_puts(fmt);                        \
> } while (0)
> 
> This fully eliminates the shadowing risk without any compile overhead.
> 
> Qian-Yu
> 
> > 
> >   
> > >  #include <linux/compiler_attributes.h>
> > >  #include <linux/instruction_pointer.h>
> > >  #include <linux/stddef.h>
> > > @@ -84,15 +85,18 @@ do {									\
> > >   * let gcc optimize the rest.
> > >   */
> > >  
> > > -#define trace_printk(fmt, ...)				\
> > > +#define ___trace_printk(fmt, str, ...)				\
> > >  do {							\
> > > -	char _______STR[] = __stringify((__VA_ARGS__));	\
> > > -	if (sizeof(_______STR) > 3)			\
> > > +	char str[] = __stringify((__VA_ARGS__));	\
> > > +	if (sizeof(str) > 3)			\
> > >  		do_trace_printk(fmt, ##__VA_ARGS__);	\
> > >  	else						\
> > >  		trace_puts(fmt);			\
> > >  } while (0)
> > >  
> > > +#define trace_printk(fmt, ...) \
> > > +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > +
> > >  #define do_trace_printk(fmt, args...)					\
> > >  do {									\
> > >  	static const char *trace_printk_fmt __used			\  
> >   

Re: [PATCH] trace_printk: replace _______STR with __UNIQUE_ID(STR)

[Qian-Yu Lin]

On Fri, May 01, 2026 at 10:13:15PM +0100, David Laight wrote:
> On Fri, 1 May 2026 22:40:17 +0800
> Qian-Yu Lin <tiffany019230@gmail.com> wrote:
> 
> ...
> > Yes. I measured compile time of kernel/trace/ring_buffer_benchmark.o
> > after make clean on an x86_64 machine running Ubuntu 24.04 LTS:
> > 
> >   - Original _______STR:                 49.8s
> >   - v1 with __UNIQUE_ID (compiler.h):    53.5s
> >   - compound literal (no extra include): 33.2s
> 
> That difference looks far to big to me.
> And the times are far too large to be measuring the actual compile time.
> 

You're right, my earlier measurements included dependency rebuilds
after make clean. I re-measured using touch to isolate the actual
compile time of ring_buffer_benchmark.o on x86_64:

  - Original ___STR:                        1.757s
  - v1 with __UNIQUE_ID (compiler.h):       1.836s
  - sizeof __stringify (your suggestion):   1.781s

> > 
> > I propose using a compound literal in v2, which eliminates the local
> > variable entirely and requires no extra include:
> > 
> > #define trace_printk(fmt, ...)                          \
> > do {                                                    \
> >     if (sizeof((char[])                             \
> >         {__stringify((__VA_ARGS__))}) > 3)      \
> >         do_trace_printk(fmt, ##__VA_ARGS__);    \
> 
> There has to be a better way to align that code.
> Although you should be able to use:
> 	if (sizeof __stringify((__VA_ARGS__)) > 3)
> (I've omitted one set of parenthesis for clarity)
> 
> You could change __stringify() to work with __VA_ARGS__ the you don't need
> the extra (); this works fine:
> #define _x(...) #__VA_ARGS__
> #define x(...) _x(__VA_ARGS__)
> #define z abcd
> int a = sizeof x(z, v); /* 8 */
> See: https://godbolt.org/z/zo4h4nr9b
> 
> -- David
> 

Yes, this works. I verified with objdump on the
samples/trace_printk module that all four cases branch correctly:
__trace_bputs, __trace_puts, __trace_bprintk, and __trace_printk.

I'll use this form in v3 since it's simpler than the compound literal.

> >     else                                            \
> >         trace_puts(fmt);                        \
> > } while (0)
> > 
> > This fully eliminates the shadowing risk without any compile overhead.
> > 
> > Qian-Yu
> > 
> > > 
> > >   
> > > >  #include <linux/compiler_attributes.h>
> > > >  #include <linux/instruction_pointer.h>
> > > >  #include <linux/stddef.h>
> > > > @@ -84,15 +85,18 @@ do {									\
> > > >   * let gcc optimize the rest.
> > > >   */
> > > >  
> > > > -#define trace_printk(fmt, ...)				\
> > > > +#define ___trace_printk(fmt, str, ...)				\
> > > >  do {							\
> > > > -	char _______STR[] = __stringify((__VA_ARGS__));	\
> > > > -	if (sizeof(_______STR) > 3)			\
> > > > +	char str[] = __stringify((__VA_ARGS__));	\
> > > > +	if (sizeof(str) > 3)			\
> > > >  		do_trace_printk(fmt, ##__VA_ARGS__);	\
> > > >  	else						\
> > > >  		trace_puts(fmt);			\
> > > >  } while (0)
> > > >  
> > > > +#define trace_printk(fmt, ...) \
> > > > +	___trace_printk(fmt, __UNIQUE_ID(str), ##__VA_ARGS__)
> > > > +
> > > >  #define do_trace_printk(fmt, args...)					\
> > > >  do {									\
> > > >  	static const char *trace_printk_fmt __used			\  
> > >   
> 

[PATCH v2] trace_printk: replace ___STR with compound literal

[Qian-Yu Lin]

The macro trace_printk() uses a hardcoded identifier ___STR
within a statement expression, which can lead to variable name
shadowing if a caller happens to use the same name in its scope.

Replace the hardcoded identifier with a compound literal, which
eliminates the local variable entirely and removes any shadowing
risk without requiring an extra include or helper macro.

Since trace_printk() is never nested, __UNIQUE_ID() provides no
practical benefit here. The compound literal approach is simpler
and has no compile time overhead, unlike the v1 approach which
added #include <linux/compiler.h>.

Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
 include/linux/trace_printk.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..7a4f6711834f 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -86,8 +86,7 @@ do {									\
 
 #define trace_printk(fmt, ...)				\
 do {							\
-	char _______STR[] = __stringify((__VA_ARGS__));	\
-	if (sizeof(_______STR) > 3)			\
+	if (sizeof((char[]){__stringify((__VA_ARGS__))}) > 3)			\
 		do_trace_printk(fmt, ##__VA_ARGS__);	\
 	else						\
 		trace_puts(fmt);			\
-- 
2.43.0

[PATCH v3] trace_printk: remove local variable for argument detection

[Qian-Yu Lin]

The trace_printk() macro uses a local variable _______STR to detect
whether variadic arguments are present. This name can shadow outer
variables.

Replace the local variable with sizeof applied directly to the
stringified arguments:

  if (sizeof __stringify((__VA_ARGS__)) > 3)

This eliminates the shadowing risk entirely without introducing
any additional includes or local variables.

Verified with objdump on samples/trace_printk that all four cases
branch correctly: __trace_bputs, __trace_puts, __trace_bprintk,
and __trace_printk.

Suggested-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Qian-Yu Lin <tiffany019230@gmail.com>
---
 include/linux/trace_printk.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/linux/trace_printk.h b/include/linux/trace_printk.h
index 2670ec7f4262..3d54f440dccf 100644
--- a/include/linux/trace_printk.h
+++ b/include/linux/trace_printk.h
@@ -86,8 +86,7 @@ do {									\
 
 #define trace_printk(fmt, ...)				\
 do {							\
-	char _______STR[] = __stringify((__VA_ARGS__));	\
-	if (sizeof(_______STR) > 3)			\
+	if (sizeof __stringify((__VA_ARGS__)) > 3)		\
 		do_trace_printk(fmt, ##__VA_ARGS__);	\
 	else						\
 		trace_puts(fmt);			\
-- 
2.43.0