[PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Shivani Bhardwaj]

Replaced the declaration of character array from char * to
static const char * const as the array is not intended to be used
anywhere outside the function plus the values in array and the pointer
to it don't have to be changed.

Signed-off-by: Shivani Bhardwaj <shivanib134@gmaail.com>
---
 drivers/staging/unisys/visorbus/visorchipset.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
index 4b76cb4..073b45b 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1411,7 +1411,7 @@ static int
 visorchipset_chipset_selftest(void)
 {
 	char env_selftest[20];
-	char *envp[] = { env_selftest, NULL };
+	static const char * const envp[] = { env_selftest, NULL };
 
 	sprintf(env_selftest, "SPARSP_SELFTEST=%d", 1);
 	kobject_uevent_env(&visorchipset_platform_device.dev.kobj, KOBJ_CHANGE,
@@ -1561,7 +1561,7 @@ parahotplug_request_kickoff(struct parahotplug_request *req)
 	struct controlvm_message_packet *cmd = &req->msg.cmd;
 	char env_cmd[40], env_id[40], env_state[40], env_bus[40], env_dev[40],
 	    env_func[40];
-	char *envp[] = {
+	static const char * const envp[] = {
 		env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
 	};
 
-- 
2.1.0

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Greg KH]

On Wed, Sep 30, 2015 at 04:54:13AM +0530, Shivani Bhardwaj wrote:
> Replaced the declaration of character array from char * to
> static const char * const as the array is not intended to be used
> anywhere outside the function plus the values in array and the pointer
> to it don't have to be changed.
> 
> Signed-off-by: Shivani Bhardwaj <shivanib134@gmaail.com>
> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c b/drivers/staging/unisys/visorbus/visorchipset.c
> index 4b76cb4..073b45b 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -1411,7 +1411,7 @@ static int
>  visorchipset_chipset_selftest(void)
>  {
>  	char env_selftest[20];
> -	char *envp[] = { env_selftest, NULL };
> +	static const char * const envp[] = { env_selftest, NULL };

The compiler didn't complain when you did this?  That's really strange,
it should have.

Think about what you just turned envp into, and how env_selftest could
ever be correct now, given that the scope lifetime rules just changed
between the two variables.

I would leave this one alone, it's tricky and odds are, something is
going to be wrong with changing it, there's nothing wrong with the
original code as-is.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Shivani Bhardwaj]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

This was a warning message by compiler only that I need to convert char *
to static const char * const. I just did that without noticing env_selftest
which is directly related. and then when I compiled, it worked. Compilers
aren't very reliable, I see.

Thanks, Greg!

On Wed, Sep 30, 2015 at 9:48 AM, Greg KH <gregkh@linuxfoundation.org> wrote:

> On Wed, Sep 30, 2015 at 09:41:14AM +0530, Shivani Bhardwaj wrote:
>
> <snip>
>
> For some reason you sent this only to me, which is a bit rude to
> everyone else on the mailing list.  I'll be glad to respond if you
> resend it to everyone.
>
> thanks,
>
> greg k-h
>

[-- Attachment #2: Type: text/html, Size: 990 bytes --]

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Shivani Bhardwaj]

[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]

This was a warning message by compiler only that I need to convert char *
to static const char * const. I just did that without noticing env_selftest
which is directly related. and then when I compiled, it worked. Compilers
aren't very reliable, I see.

Thanks, Greg!

On Wed, Sep 30, 2015 at 9:50 AM, Shivani Bhardwaj <shivanib134@gmail.com>
wrote:

> This was a warning message by compiler only that I need to convert char *
> to static const char * const. I just did that without noticing env_selftest
> which is directly related. and then when I compiled, it worked. Compilers
> aren't very reliable, I see.
>
> Thanks, Greg!
>
> On Wed, Sep 30, 2015 at 9:48 AM, Greg KH <gregkh@linuxfoundation.org>
> wrote:
>
>> On Wed, Sep 30, 2015 at 09:41:14AM +0530, Shivani Bhardwaj wrote:
>>
>> <snip>
>>
>> For some reason you sent this only to me, which is a bit rude to
>> everyone else on the mailing list.  I'll be glad to respond if you
>> resend it to everyone.
>>
>> thanks,
>>
>> greg k-h
>>
>
>

[-- Attachment #2: Type: text/html, Size: 1805 bytes --]

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Vaishali Thakkar]

On Wed, Sep 30, 2015 at 9:56 AM, Shivani Bhardwaj <shivanib134@gmail.com> wrote:
> This was a warning message by compiler only that I need to convert char * to
> static const char * const. I just did that without noticing env_selftest
> which is directly related. and then when I compiled, it worked. Compilers
> aren't very reliable, I see.

Please do not top-post. When you are responding to email, respond inline.
You can use this link to understand proper communication style:
http://kernelnewbies.org/FirstKernelPatch#head-9799d3ebaa1221875dfdb0a05c4063dc4eb474a1

> Thanks, Greg!
>
> On Wed, Sep 30, 2015 at 9:50 AM, Shivani Bhardwaj <shivanib134@gmail.com>
> wrote:
>>
>> This was a warning message by compiler only that I need to convert char *
>> to static const char * const. I just did that without noticing env_selftest
>> which is directly related. and then when I compiled, it worked. Compilers
>> aren't very reliable, I see.
>>
>> Thanks, Greg!
>>
>> On Wed, Sep 30, 2015 at 9:48 AM, Greg KH <gregkh@linuxfoundation.org>
>> wrote:
>>>
>>> On Wed, Sep 30, 2015 at 09:41:14AM +0530, Shivani Bhardwaj wrote:
>>>
>>> <snip>
>>>
>>> For some reason you sent this only to me, which is a bit rude to
>>> everyone else on the mailing list.  I'll be glad to respond if you
>>> resend it to everyone.
>>>
>>> thanks,
>>>
>>> greg k-h
>>
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/CAKHNQQGy1mPrWEcWqHFBNz_8BiG2sFSGAL9LmHgSLS0gPn9cog%40mail.gmail.com.
>
> For more options, visit https://groups.google.com/d/optout.



-- 
Vaishali

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Greg KH]

On Wed, Sep 30, 2015 at 09:50:50AM +0530, Shivani Bhardwaj wrote:
> This was a warning message by compiler only that I need to convert char * to
> static const char * const.

The compiler said that?  Can you post the exact message gcc gave you for
this line, that seems very odd.

thanks,

greg k-h

Re: [Outreachy kernel] [PATCH] Staging: unisys: visorbus: visorchipset: Converted arrays to static const

[Shivani Bhardwaj]

[-- Attachment #1: Type: text/plain, Size: 3283 bytes --]

Thanks Julia!
And Greg, I'm really sorry, warning was generated by checkpatch script not
gcc. I read online that it does generate false warnings. Thank you.

On Wed, Sep 30, 2015 at 11:34 PM, Shivani Bhardwaj <shivanib134@gmail.com>
wrote:

> Thanks Julia!
> And Greg, I'm really sorry, warning was generated by checkpatch script not
> gcc. I read online that it does generate false warnings. Thank you.
>
>
> On Wed, Sep 30, 2015 at 10:56 AM, Julia Lawall <julia.lawall@lip6.fr>
> wrote:
>
>> On Wed, 30 Sep 2015, Shivani Bhardwaj wrote:
>>
>> > Replaced the declaration of character array from char * to
>> > static const char * const as the array is not intended to be used
>> > anywhere outside the function plus the values in array and the pointer
>> > to it don't have to be changed.
>>
>> It's confusing, but static should not be used on local variable because
>> they are not used outside the function.  Static means that the variable is
>> allocated only once over all executions of the functions.  Actually, it
>> becomes like a global variable that no one else can see.  If you call the
>> function once, and change the value of the variable, then on the next call
>> you see the changed value until the variable is modified.
>>
>> Here that all makes no sense, because you say that the array is not
>> modified, and anyway the code explicitly sets the variable to its original
>> value.
>>
>> julia
>>
>> > Signed-off-by: Shivani Bhardwaj <shivanib134@gmaail.com>
>> > ---
>> >  drivers/staging/unisys/visorbus/visorchipset.c | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
>> b/drivers/staging/unisys/visorbus/visorchipset.c
>> > index 4b76cb4..073b45b 100644
>> > --- a/drivers/staging/unisys/visorbus/visorchipset.c
>> > +++ b/drivers/staging/unisys/visorbus/visorchipset.c
>> > @@ -1411,7 +1411,7 @@ static int
>> >  visorchipset_chipset_selftest(void)
>> >  {
>> >       char env_selftest[20];
>> > -     char *envp[] = { env_selftest, NULL };
>> > +     static const char * const envp[] = { env_selftest, NULL };
>> >
>> >       sprintf(env_selftest, "SPARSP_SELFTEST=%d", 1);
>> >       kobject_uevent_env(&visorchipset_platform_device.dev.kobj,
>> KOBJ_CHANGE,
>> > @@ -1561,7 +1561,7 @@ parahotplug_request_kickoff(struct
>> parahotplug_request *req)
>> >       struct controlvm_message_packet *cmd = &req->msg.cmd;
>> >       char env_cmd[40], env_id[40], env_state[40], env_bus[40],
>> env_dev[40],
>> >           env_func[40];
>> > -     char *envp[] = {
>> > +     static const char * const envp[] = {
>> >               env_cmd, env_id, env_state, env_bus, env_dev, env_func,
>> NULL
>> >       };
>> >
>> > --
>> > 2.1.0
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups "outreachy-kernel" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> > To post to this group, send email to outreachy-kernel@googlegroups.com.
>> > To view this discussion on the web visit
>> https://groups.google.com/d/msgid/outreachy-kernel/20150929232413.GA16457%40ubuntu
>> .
>> > For more options, visit https://groups.google.com/d/optout.
>> >
>>
>
>

[-- Attachment #2: Type: text/html, Size: 4776 bytes --]