Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Date:   Mon, 2 Oct 2023 10:33:49 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Alexey Dobriyan <adobriyan@gmail.com>
Cc:     akpm@linux-foundation.org, Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.de>
Subject: Re: [PATCH] extract and use FILE_LINE macro
Message-ID: <202310021033.A5F0159F25@keescook>
References: <ebf12ac4-5a61-4b12-b8b0-1253eb371332@p183>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ebf12ac4-5a61-4b12-b8b0-1253eb371332@p183>
Precedence: bulk

On Sat, Sep 16, 2023 at 09:21:31PM +0300, Alexey Dobriyan wrote:
> Extract nifty FILE_LINE useful for printk style debugging:
> 
> 	printk("%s\n", FILE_LINE);
> 
> 
> It should not be used en mass probably because __FILE__ string literals
> can be merged while FILE_LINE's won't. But for debugging it is what
> the doctor ordered.
> 
> Don't add leading and trailing underscores, they're painful to type.
> Trust me, I've tried both versions.
> 
> Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> ---
> 
>  include/linux/fortify-string.h |    2 +-
>  include/linux/stringify.h      |    2 ++
>  include/linux/timer.h          |    3 +--
>  sound/pci/asihpi/hpidebug.h    |    9 ++++-----
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> --- a/include/linux/fortify-string.h
> +++ b/include/linux/fortify-string.h
> @@ -643,7 +643,7 @@ __FORTIFY_INLINE bool fortify_memcpy_chk(__kernel_size_t size,
>  				     __q_size_field, #op),		\
>  		  #op ": detected field-spanning write (size %zu) of single %s (size %zu)\n", \
>  		  __fortify_size,					\
> -		  "field \"" #p "\" at " __FILE__ ":" __stringify(__LINE__), \
> +		  "field \"" #p "\" at " FILE_LINE,			\
>  		  __p_size_field);					\
>  	__underlying_##op(p, q, __fortify_size);			\
>  })
> --- a/include/linux/stringify.h
> +++ b/include/linux/stringify.h
> @@ -9,4 +9,6 @@
>  #define __stringify_1(x...)	#x
>  #define __stringify(x...)	__stringify_1(x)
>  
> +#define FILE_LINE	__FILE__ ":" __stringify(__LINE__)
> +
>  #endif	/* !__LINUX_STRINGIFY_H */
> --- a/include/linux/timer.h
> +++ b/include/linux/timer.h
> @@ -77,8 +77,7 @@ struct timer_list {
>  		.entry = { .next = TIMER_ENTRY_STATIC },	\
>  		.function = (_function),			\
>  		.flags = (_flags),				\
> -		__TIMER_LOCKDEP_MAP_INITIALIZER(		\
> -			__FILE__ ":" __stringify(__LINE__))	\
> +		__TIMER_LOCKDEP_MAP_INITIALIZER(FILE_LINE)	\
>  	}
>  
>  #define DEFINE_TIMER(_name, _function)				\
> --- a/sound/pci/asihpi/hpidebug.h
> +++ b/sound/pci/asihpi/hpidebug.h
> @@ -29,16 +29,15 @@ enum { HPI_DEBUG_LEVEL_ERROR = 0,	/* always log errors */
>     the start of each message, eg see linux kernel hpios.h */
>  
>  #ifdef SOURCEFILE_NAME
> +#undef FILE_LINE
>  #define FILE_LINE  SOURCEFILE_NAME ":" __stringify(__LINE__) " "

Should this drop the trailing " " to match the new macro?

-Kees

> -#else
> -#define FILE_LINE  __FILE__ ":" __stringify(__LINE__) " "
>  #endif
>  
>  #define HPI_DEBUG_ASSERT(expression) \
>  	do { \
>  		if (!(expression)) { \
>  			printk(KERN_ERR  FILE_LINE \
> -				"ASSERT " __stringify(expression)); \
> +				" ASSERT " __stringify(expression)); \
>  		} \
>  	} while (0)
>  
> @@ -46,7 +45,7 @@ enum { HPI_DEBUG_LEVEL_ERROR = 0,	/* always log errors */
>  	do { \
>  		if (hpi_debug_level >= HPI_DEBUG_LEVEL_##level) { \
>  			printk(HPI_DEBUG_FLAG_##level \
> -			FILE_LINE  __VA_ARGS__); \
> +			FILE_LINE " " __VA_ARGS__); \
>  		} \
>  	} while (0)
>  
> @@ -70,7 +69,7 @@ void hpi_debug_data(u16 *pdata, u32 len);
>  	do { \
>  		if (hpi_debug_level >= HPI_DEBUG_LEVEL_##level) { \
>  			hpi_debug_message(phm, HPI_DEBUG_FLAG_##level \
> -				FILE_LINE __stringify(level)); \
> +				FILE_LINE " " __stringify(level)); \
>  		} \
>  	} while (0)
>  

-- 
Kees Cook