linux-staging.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: greybus: audio_manager_module: make envp array static const
@ 2025-11-13 12:52 Chang Junzheng
  0 siblings, 0 replies; 5+ messages in thread
From: Chang Junzheng @ 2025-11-13 12:52 UTC (permalink / raw)
  To: outreachy
  Cc: Greg Kroah-Hartman, linux-staging, linux-kernel, guagua210311,
	Chang Junzheng

From: Chang Junzheng <guagua210311@outlook.com>

The envp array in send_add_uevent() function is declared as a non-const
local array, which triggers the following checkpatch.pl warning:

WARNING: char * array declaration might be better as static const

Change the declaration to 'static const char * const' to improve code
safety by making the array read-only and allow for better compiler
optimization. This follows the kernel coding style recommendations.

Signed-off-by: Chang Junzheng <guagua210311@qq.com>
---
 drivers/staging/greybus/audio_manager_module.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
index 4a4dfb42f50f..ca6a2cd0bc4f 100644
--- a/drivers/staging/greybus/audio_manager_module.c
+++ b/drivers/staging/greybus/audio_manager_module.c
@@ -159,14 +159,14 @@ static void send_add_uevent(struct gb_audio_manager_module *module)
 	char ip_devices_string[64];
 	char op_devices_string[64];
 
-	char *envp[] = {
-		name_string,
-		vid_string,
-		pid_string,
-		intf_id_string,
-		ip_devices_string,
-		op_devices_string,
-		NULL
+	static const char * const envp[] = {
+						name_string,
+						vid_string,
+						pid_string,
+						intf_id_string,
+						ip_devices_string,
+						op_devices_string,
+						NULL
 	};
 
 	snprintf(name_string, 128, "NAME=%s", module->desc.name);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH] staging: greybus: audio_manager_module: make envp array static const
@ 2025-11-13 13:01 Chang Junzheng
  2025-11-13 13:20 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: Chang Junzheng @ 2025-11-13 13:01 UTC (permalink / raw)
  To: outreachy
  Cc: Vaibhav Agarwal, Mark Greer, Johan Hovold, Alex Elder,
	Greg Kroah-Hartman, greybus-dev, linux-staging, linux-kernel,
	guagua210311, Chang Junzheng

From: Chang Junzheng <guagua210311@outlook.com>

The envp array in send_add_uevent() function is declared as a non-const
local array, which triggers the following checkpatch.pl warning:

WARNING: char * array declaration might be better as static const

Change the declaration to 'static const char * const' to improve code
safety by making the array read-only and allow for better compiler
optimization. This follows the kernel coding style recommendations.

Signed-off-by: Chang Junzheng <guagua210311@qq.com>
---
 drivers/staging/greybus/audio_manager_module.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
index 4a4dfb42f50f..ca6a2cd0bc4f 100644
--- a/drivers/staging/greybus/audio_manager_module.c
+++ b/drivers/staging/greybus/audio_manager_module.c
@@ -159,14 +159,14 @@ static void send_add_uevent(struct gb_audio_manager_module *module)
 	char ip_devices_string[64];
 	char op_devices_string[64];
 
-	char *envp[] = {
-		name_string,
-		vid_string,
-		pid_string,
-		intf_id_string,
-		ip_devices_string,
-		op_devices_string,
-		NULL
+	static const char * const envp[] = {
+						name_string,
+						vid_string,
+						pid_string,
+						intf_id_string,
+						ip_devices_string,
+						op_devices_string,
+						NULL
 	};
 
 	snprintf(name_string, 128, "NAME=%s", module->desc.name);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
  2025-11-13 13:01 [PATCH] staging: greybus: audio_manager_module: make envp array static const Chang Junzheng
@ 2025-11-13 13:20 ` Greg Kroah-Hartman
  2025-11-13 13:39   ` cjz_VM
  0 siblings, 1 reply; 5+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-13 13:20 UTC (permalink / raw)
  To: Chang Junzheng
  Cc: outreachy, Vaibhav Agarwal, Mark Greer, Johan Hovold, Alex Elder,
	greybus-dev, linux-staging, linux-kernel, Chang Junzheng

On Thu, Nov 13, 2025 at 09:01:46PM +0800, Chang Junzheng wrote:
> From: Chang Junzheng <guagua210311@outlook.com>
> 
> The envp array in send_add_uevent() function is declared as a non-const
> local array, which triggers the following checkpatch.pl warning:
> 
> WARNING: char * array declaration might be better as static const
> 
> Change the declaration to 'static const char * const' to improve code
> safety by making the array read-only and allow for better compiler
> optimization. This follows the kernel coding style recommendations.
> 
> Signed-off-by: Chang Junzheng <guagua210311@qq.com>

You sent this twice?

> ---
>  drivers/staging/greybus/audio_manager_module.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_manager_module.c b/drivers/staging/greybus/audio_manager_module.c
> index 4a4dfb42f50f..ca6a2cd0bc4f 100644
> --- a/drivers/staging/greybus/audio_manager_module.c
> +++ b/drivers/staging/greybus/audio_manager_module.c
> @@ -159,14 +159,14 @@ static void send_add_uevent(struct gb_audio_manager_module *module)
>  	char ip_devices_string[64];
>  	char op_devices_string[64];
>  
> -	char *envp[] = {
> -		name_string,
> -		vid_string,
> -		pid_string,
> -		intf_id_string,
> -		ip_devices_string,
> -		op_devices_string,
> -		NULL
> +	static const char * const envp[] = {
> +						name_string,
> +						vid_string,
> +						pid_string,
> +						intf_id_string,
> +						ip_devices_string,
> +						op_devices_string,
> +						NULL

Why did you indent this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
  2025-11-13 13:20 ` Greg Kroah-Hartman
@ 2025-11-13 13:39   ` cjz_VM
  2025-11-13 13:46     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 5+ messages in thread
From: cjz_VM @ 2025-11-13 13:39 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: outreachy, Vaibhav Agarwal, Mark Greer, Johan Hovold, Alex Elder,
	greybus-dev, linux-staging, linux-kernel, guagua210311

Hi Greg,

Thanks for reviewing my patch!

For sending twice: I apologize for the duplicate. After the first send, I realized I had missed some greybus-specific maintainers, so I resent to include them all.

For the indentation: You're right to ask about it. When I changed the declaration from 'char *envp[]' to 'static const char * const envp[]', the opening bracket moved to the right due to the longer declaration. I added tabs to keep the array elements aligned with the opening bracket, following the kernel coding style rule that parameters should align with the opening parenthesis.

If this alignment approach is not preferred, I'm happy to adjust it to whatever style you recommend.

Thanks again for your time and guidance!

Best regards,
Chang JunzhengFrom: cjz_VM <guagua210311@qq.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: outreachy@lists.linux.dev, Vaibhav Agarwal <vaibhav.sr@gmail.com>, Mark Greer <mgreer@animalcreek.com>, Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>, greybus-dev@lists.linaro.org, linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org, guagua210311@qq.com
Subject: Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
Reply-To: 
In-Reply-To: <2025111341-attendee-ferment-262b@gregkh>

Hi Greg,

Thanks for reviewing my patch!

For sending twice: I apologize for the duplicate. After the first send, I realized I had missed some greybus-specific maintainers, so I resent to include them all.

For the indentation: You're right to ask about it. When I changed the declaration from 'char *envp[]' to 'static const char * const envp[]', the opening bracket moved to the right due to the longer declaration. I added tabs to keep the array elements aligned with the opening bracket, following the kernel coding style rule that parameters should align with the opening parenthesis.

If this alignment approach is not preferred, I'm happy to adjust it to whatever style you recommend.

Thanks again for your time and guidance!

Best regards,
Chang Junzheng


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
  2025-11-13 13:39   ` cjz_VM
@ 2025-11-13 13:46     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2025-11-13 13:46 UTC (permalink / raw)
  To: cjz_VM
  Cc: outreachy, Vaibhav Agarwal, Mark Greer, Johan Hovold, Alex Elder,
	greybus-dev, linux-staging, linux-kernel

On Thu, Nov 13, 2025 at 09:39:54PM +0800, cjz_VM wrote:
> Hi Greg,
> 
> Thanks for reviewing my patch!
> 
> For sending twice: I apologize for the duplicate. After the first send, I realized I had missed some greybus-specific maintainers, so I resent to include them all.

Then that should have been a v2 patch :)

> For the indentation: You're right to ask about it. When I changed the declaration from 'char *envp[]' to 'static const char * const envp[]', the opening bracket moved to the right due to the longer declaration. I added tabs to keep the array elements aligned with the opening bracket, following the kernel coding style rule that parameters should align with the opening parenthesis.
> 
> If this alignment approach is not preferred, I'm happy to adjust it to whatever style you recommend.

You did not document your change, and it had nothing really to do with
the const change, so I would not recommend doing this.

Also, please do not top-post, and be sure to properly wrap your lines in
your email client.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-13 13:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-13 13:01 [PATCH] staging: greybus: audio_manager_module: make envp array static const Chang Junzheng
2025-11-13 13:20 ` Greg Kroah-Hartman
2025-11-13 13:39   ` cjz_VM
2025-11-13 13:46     ` Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2025-11-13 12:52 Chang Junzheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).