Re: [PATCH v1 1/1] panic: Fix compilation error (`make W=1`)

[Nathan Chancellor <nathan@kernel.org>]

On Fri, Jul 11, 2025 at 08:54:25AM +0800, Feng Tang wrote:
> On Thu, Jul 10, 2025 at 03:01:33PM -0700, Andrew Morton wrote:
> > On Thu, 10 Jul 2025 12:48:16 +0300 Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> > > Compiler is not happy about the recently added code:
> > > 
> > > lib/sys_info.c:52:19: error: variable 'sys_info_avail' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]
> > >    52 | static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
> > >       |                   ^~~~~~~~~~~~~~
> > > 
> > > Fix it in the same way how, for example, lib/vsprintf.c does in the similar
> > > cases, i.e. by using string literal directly as sizeof() parameter.
> > > 
> > > ...
> > >
> > 
> > > --- a/lib/sys_info.c
> > > +++ b/lib/sys_info.c
> > > @@ -49,13 +49,11 @@ unsigned long sys_info_parse_param(char *str)
> > >  
> > >  #ifdef CONFIG_SYSCTL
> > >  
> > > -static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
> > > -
> > >  int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > >  					  void *buffer, size_t *lenp,
> > >  					  loff_t *ppos)
> > >  {
> > > -	char names[sizeof(sys_info_avail) + 1];
> > > +	char names[sizeof("tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks") + 1];
> > >  	struct ctl_table table;
> > >  	unsigned long *si_bits_global;
> > >  
> > 
> > Yes, that's neater than the fix we currently have.  I'll grab, thanks.
> 
> Hi Andrew, Andy,
> 
> sys_info_avail[] has another purpose for being a counterpart of si_names[],
> which could be extended in future, so we make it obviously stand-alone. As
> for definition of si_names[], we explicitly added comment:  
> 
> 	/*
> 	 * When 'si_names' gets updated,  please make sure the 'sys_info_avail'
> 	 * below is updated accordingly.
> 	 */
> 	static const struct sys_info_name  si_names[] = {
> 		{ SYS_INFO_TASKS,		"tasks" },
> 		{ SYS_INFO_MEM,			"mem" },
> 		
> which has also been discussed in another thread:
> https://lore.kernel.org/lkml/aG3o2RFHc5iXnJef@U-2FWC9VHC-2323.local/
> 
> And I suggest to keep sys_info_avail[], and either Nathan or Sergey's patch
> works for me.

We could do something like this to keep the sizeof() obvious and
separate, while still eliminating the variable? Happy to bike shed
aspects of it like the macro name and such.

diff --git a/lib/sys_info.c b/lib/sys_info.c
index 46d6f4f1ad2a..c1df502a2c0d 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -14,7 +14,7 @@ struct sys_info_name {
 };
 
 /*
- * When 'si_names' gets updated,  please make sure the 'sys_info_avail'
+ * When 'si_names' gets updated,  please make sure SYS_INFO_MAX_LEN
  * below is updated accordingly.
  */
 static const struct sys_info_name  si_names[] = {
@@ -49,13 +49,13 @@ unsigned long sys_info_parse_param(char *str)
 
 #ifdef CONFIG_SYSCTL
 
-static const char sys_info_avail[] = "tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks";
+#define SYS_INFO_MAX_LEN (sizeof("tasks,mem,timers,locks,ftrace,all_bt,blocked_tasks") + 1)
 
 int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
 					  void *buffer, size_t *lenp,
 					  loff_t *ppos)
 {
-	char names[sizeof(sys_info_avail) + 1];
+	char names[SYS_INFO_MAX_LEN];
 	struct ctl_table table;
 	unsigned long *si_bits_global;
 
---

> Sorry for the inconvenience, and I should upgrade my gcc :) 

I am not sure that GCC has this warning, I have only ever seen it with
clang.

Cheers,
Nathan