[PATCH] staging: greybus: audio_manager_module: make envp array static const

Chang Junzheng posted 1 patch 2 months, 3 weeks ago
There is a newer version of this series
drivers/staging/greybus/audio_manager_module.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
[PATCH] staging: greybus: audio_manager_module: make envp array static const
Posted by Chang Junzheng 2 months, 3 weeks ago
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
Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
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
Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
Posted by cjz_VM 2 months, 3 weeks ago
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
Re: [PATCH] staging: greybus: audio_manager_module: make envp array static const
Posted by Greg Kroah-Hartman 2 months, 3 weeks ago
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