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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250711014947.GA863150@ax162>
Date: Thu, 10 Jul 2025 18:49:47 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Feng Tang <feng.tang@...ux.alibaba.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	linux-kernel@...r.kernel.org,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Petr Mladek <pmladek@...e.com>
Subject: Re: [PATCH v1 1/1] panic: Fix compilation error (`make W=1`)

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@...ux.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