2017-09-09 07:03:05

by Harsha Sharma

[permalink] [raw]
Subject: [PATCH] staging: unisys: visorbus: Declared char * array as static const

State explicitly that individual entries in array will not change.

Signed-off-by: Harsha Sharma <[email protected]>
---
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 6d4498f..6f2a010 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1162,7 +1162,7 @@ static ssize_t deviceenabled_store(struct device *dev,
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
};

@@ -1263,7 +1263,7 @@ static ssize_t deviceenabled_store(struct device *dev,
chipset_selftest_uevent(struct controlvm_message_header *msg_hdr)
{
char env_selftest[20];
- char *envp[] = { env_selftest, NULL };
+ static const char * const envp[] = { env_selftest, NULL };
int res;

sprintf(env_selftest, "SPARSP_SELFTEST=%d", 1);
--
1.9.1


2017-09-09 07:14:55

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const

On Sat, Sep 09, 2017 at 12:30:42PM +0530, Harsha Sharma wrote:
> State explicitly that individual entries in array will not change.
>
> Signed-off-by: Harsha Sharma <[email protected]>
> ---
> 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 6d4498f..6f2a010 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -1162,7 +1162,7 @@ static ssize_t deviceenabled_store(struct device *dev,
> 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[] = {

Are you _sure_ about this? Why make it static? That seems a bit "odd",
don't you think? You need a lot more changelog text to get everyone to
agree that this is ok to do...

Also, you forgot to cc: the actual maintainers of this code...

thanks,

greg k-h

2017-09-09 08:36:13

by Julia Lawall

[permalink] [raw]
Subject: Re: [Outreachy kernel] Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const



On Sat, 9 Sep 2017, Greg KH wrote:

> On Sat, Sep 09, 2017 at 12:30:42PM +0530, Harsha Sharma wrote:
> > State explicitly that individual entries in array will not change.
> >
> > Signed-off-by: Harsha Sharma <[email protected]>
> > ---
> > 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 6d4498f..6f2a010 100644
> > --- a/drivers/staging/unisys/visorbus/visorchipset.c
> > +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> > @@ -1162,7 +1162,7 @@ static ssize_t deviceenabled_store(struct device *dev,
> > 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[] = {
>
> Are you _sure_ about this? Why make it static? That seems a bit "odd",
> don't you think? You need a lot more changelog text to get everyone to
> agree that this is ok to do...

Harsha,

Study carefully what static means when it is attached to a local variable.
And be sure that your code actually compiles. If it doesn't try to figure
out why not. There are other commits that are sort of like this on in the
kernel that you can find using git log. But you may notice that they are
different in some way from yours.

julia


>
> Also, you forgot to cc: the actual maintainers of this code...
>
> 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 [email protected].
> To post to this group, send email to [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170909071451.GA27010%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
>

2017-09-11 09:30:34

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const

Hi Harsha,

[auto build test ERROR on v4.13]
[also build test ERROR on next-20170911]
[cannot apply to staging/staging-testing]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Harsha-Sharma/staging-unisys-visorbus-Declared-char-array-as-static-const/20170911-161501
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All errors (new ones prefixed by >>):

drivers/staging/unisys/visorbus/visorchipset.c: In function 'parahotplug_request_kickoff':
>> drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: note: (near initialization for 'envp[0]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:12: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:12: note: (near initialization for 'envp[1]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:20: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:20: note: (near initialization for 'envp[2]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:31: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:31: note: (near initialization for 'envp[3]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:40: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:40: note: (near initialization for 'envp[4]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:49: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^~~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1174:49: note: (near initialization for 'envp[5]')
>> drivers/staging/unisys/visorbus/visorchipset.c:1189:20: error: passing argument 3 of 'kobject_uevent_env' from incompatible pointer type [-Werror=incompatible-pointer-types]
KOBJ_CHANGE, envp);
^~~~
In file included from include/linux/device.h:17:0,
from include/linux/acpi.h:27,
from drivers/staging/unisys/visorbus/visorchipset.c:17:
include/linux/kobject.h:218:5: note: expected 'char **' but argument is of type 'const char * const*'
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
^~~~~~~~~~~~~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c: In function 'chipset_selftest_uevent':
drivers/staging/unisys/visorbus/visorchipset.c:1275:39: error: initializer element is not constant
static const char * const envp[] = { env_selftest, NULL };
^~~~~~~~~~~~
drivers/staging/unisys/visorbus/visorchipset.c:1275:39: note: (near initialization for 'envp[0]')
drivers/staging/unisys/visorbus/visorchipset.c:1280:19: error: passing argument 3 of 'kobject_uevent_env' from incompatible pointer type [-Werror=incompatible-pointer-types]
KOBJ_CHANGE, envp);
^~~~
In file included from include/linux/device.h:17:0,
from include/linux/acpi.h:27,
from drivers/staging/unisys/visorbus/visorchipset.c:17:
include/linux/kobject.h:218:5: note: expected 'char **' but argument is of type 'const char * const*'
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
^~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors

vim +1174 drivers/staging/unisys/visorbus/visorchipset.c

ebeff055 David Kershner 2016-09-19 1159
04dbfea6 David Binder 2017-02-21 1160 /*
ebeff055 David Kershner 2016-09-19 1161 * parahotplug_request_kickoff() - initiate parahotplug request
ebeff055 David Kershner 2016-09-19 1162 * @req: the request to initiate
ebeff055 David Kershner 2016-09-19 1163 *
ebeff055 David Kershner 2016-09-19 1164 * Cause uevent to run the user level script to do the disable/enable specified
ebeff055 David Kershner 2016-09-19 1165 * in the parahotplug_request.
ebeff055 David Kershner 2016-09-19 1166 */
ae0fa822 David Kershner 2017-03-28 1167 static int
ebeff055 David Kershner 2016-09-19 1168 parahotplug_request_kickoff(struct parahotplug_request *req)
ebeff055 David Kershner 2016-09-19 1169 {
ebeff055 David Kershner 2016-09-19 1170 struct controlvm_message_packet *cmd = &req->msg.cmd;
ebeff055 David Kershner 2016-09-19 1171 char env_cmd[40], env_id[40], env_state[40], env_bus[40], env_dev[40],
ebeff055 David Kershner 2016-09-19 1172 env_func[40];
fdf58f16 Harsha Sharma 2017-09-09 1173 static const char * const envp[] = {
ebeff055 David Kershner 2016-09-19 @1174 env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
ebeff055 David Kershner 2016-09-19 1175 };
ebeff055 David Kershner 2016-09-19 1176
c5a28902 Sameer Wadgaonkar 2017-05-19 1177 sprintf(env_cmd, "VISOR_PARAHOTPLUG=1");
c5a28902 Sameer Wadgaonkar 2017-05-19 1178 sprintf(env_id, "VISOR_PARAHOTPLUG_ID=%d", req->id);
c5a28902 Sameer Wadgaonkar 2017-05-19 1179 sprintf(env_state, "VISOR_PARAHOTPLUG_STATE=%d",
ebeff055 David Kershner 2016-09-19 1180 cmd->device_change_state.state.active);
c5a28902 Sameer Wadgaonkar 2017-05-19 1181 sprintf(env_bus, "VISOR_PARAHOTPLUG_BUS=%d",
ebeff055 David Kershner 2016-09-19 1182 cmd->device_change_state.bus_no);
c5a28902 Sameer Wadgaonkar 2017-05-19 1183 sprintf(env_dev, "VISOR_PARAHOTPLUG_DEVICE=%d",
ebeff055 David Kershner 2016-09-19 1184 cmd->device_change_state.dev_no >> 3);
c5a28902 Sameer Wadgaonkar 2017-05-19 1185 sprintf(env_func, "VISOR_PARAHOTPLUG_FUNCTION=%d",
ebeff055 David Kershner 2016-09-19 1186 cmd->device_change_state.dev_no & 0x7);
ebeff055 David Kershner 2016-09-19 1187
ae0fa822 David Kershner 2017-03-28 1188 return kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj,
ae0fa822 David Kershner 2017-03-28 @1189 KOBJ_CHANGE, envp);
ebeff055 David Kershner 2016-09-19 1190 }
ebeff055 David Kershner 2016-09-19 1191

:::::: The code at line 1174 was first introduced by commit
:::::: ebeff0558c0a311f4c5d432c69c32b9502219190 staging: unisys: visorbus: move deviceenabled/disabled store functions

:::::: TO: David Kershner <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (7.16 kB)
.config.gz (58.88 kB)
Download all attachments

2017-09-12 00:29:14

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const

Hi Harsha,

[auto build test WARNING on v4.13]
[also build test WARNING on next-20170911]
[cannot apply to staging/staging-testing]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Harsha-Sharma/staging-unisys-visorbus-Declared-char-array-as-static-const/20170911-161501
config: x86_64-randconfig-it0-09120552 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64

All warnings (new ones prefixed by >>):

drivers/staging/unisys/visorbus/visorchipset.c: In function 'parahotplug_request_kickoff':
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
^
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[0]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[1]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[2]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[3]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[4]')
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: initializer element is not constant
drivers/staging/unisys/visorbus/visorchipset.c:1174:3: error: (near initialization for 'envp[5]')
>> drivers/staging/unisys/visorbus/visorchipset.c:1189:20: warning: passing argument 3 of 'kobject_uevent_env' from incompatible pointer type
KOBJ_CHANGE, envp);
^
In file included from include/linux/device.h:17:0,
from include/linux/acpi.h:27,
from drivers/staging/unisys/visorbus/visorchipset.c:17:
include/linux/kobject.h:218:5: note: expected 'char **' but argument is of type 'const char * const*'
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
^
drivers/staging/unisys/visorbus/visorchipset.c: In function 'chipset_selftest_uevent':
drivers/staging/unisys/visorbus/visorchipset.c:1275:2: error: initializer element is not constant
static const char * const envp[] = { env_selftest, NULL };
^
drivers/staging/unisys/visorbus/visorchipset.c:1275:2: error: (near initialization for 'envp[0]')
drivers/staging/unisys/visorbus/visorchipset.c:1280:19: warning: passing argument 3 of 'kobject_uevent_env' from incompatible pointer type
KOBJ_CHANGE, envp);
^
In file included from include/linux/device.h:17:0,
from include/linux/acpi.h:27,
from drivers/staging/unisys/visorbus/visorchipset.c:17:
include/linux/kobject.h:218:5: note: expected 'char **' but argument is of type 'const char * const*'
int kobject_uevent_env(struct kobject *kobj, enum kobject_action action,
^

vim +/kobject_uevent_env +1189 drivers/staging/unisys/visorbus/visorchipset.c

ebeff055 David Kershner 2016-09-19 1159
04dbfea6 David Binder 2017-02-21 1160 /*
ebeff055 David Kershner 2016-09-19 1161 * parahotplug_request_kickoff() - initiate parahotplug request
ebeff055 David Kershner 2016-09-19 1162 * @req: the request to initiate
ebeff055 David Kershner 2016-09-19 1163 *
ebeff055 David Kershner 2016-09-19 1164 * Cause uevent to run the user level script to do the disable/enable specified
ebeff055 David Kershner 2016-09-19 1165 * in the parahotplug_request.
ebeff055 David Kershner 2016-09-19 1166 */
ae0fa822 David Kershner 2017-03-28 1167 static int
ebeff055 David Kershner 2016-09-19 1168 parahotplug_request_kickoff(struct parahotplug_request *req)
ebeff055 David Kershner 2016-09-19 1169 {
ebeff055 David Kershner 2016-09-19 1170 struct controlvm_message_packet *cmd = &req->msg.cmd;
ebeff055 David Kershner 2016-09-19 1171 char env_cmd[40], env_id[40], env_state[40], env_bus[40], env_dev[40],
ebeff055 David Kershner 2016-09-19 1172 env_func[40];
fdf58f16 Harsha Sharma 2017-09-09 1173 static const char * const envp[] = {
ebeff055 David Kershner 2016-09-19 @1174 env_cmd, env_id, env_state, env_bus, env_dev, env_func, NULL
ebeff055 David Kershner 2016-09-19 1175 };
ebeff055 David Kershner 2016-09-19 1176
c5a28902 Sameer Wadgaonkar 2017-05-19 1177 sprintf(env_cmd, "VISOR_PARAHOTPLUG=1");
c5a28902 Sameer Wadgaonkar 2017-05-19 1178 sprintf(env_id, "VISOR_PARAHOTPLUG_ID=%d", req->id);
c5a28902 Sameer Wadgaonkar 2017-05-19 1179 sprintf(env_state, "VISOR_PARAHOTPLUG_STATE=%d",
ebeff055 David Kershner 2016-09-19 1180 cmd->device_change_state.state.active);
c5a28902 Sameer Wadgaonkar 2017-05-19 1181 sprintf(env_bus, "VISOR_PARAHOTPLUG_BUS=%d",
ebeff055 David Kershner 2016-09-19 1182 cmd->device_change_state.bus_no);
c5a28902 Sameer Wadgaonkar 2017-05-19 1183 sprintf(env_dev, "VISOR_PARAHOTPLUG_DEVICE=%d",
ebeff055 David Kershner 2016-09-19 1184 cmd->device_change_state.dev_no >> 3);
c5a28902 Sameer Wadgaonkar 2017-05-19 1185 sprintf(env_func, "VISOR_PARAHOTPLUG_FUNCTION=%d",
ebeff055 David Kershner 2016-09-19 1186 cmd->device_change_state.dev_no & 0x7);
ebeff055 David Kershner 2016-09-19 1187
ae0fa822 David Kershner 2017-03-28 1188 return kobject_uevent_env(&chipset_dev->acpi_device->dev.kobj,
ae0fa822 David Kershner 2017-03-28 @1189 KOBJ_CHANGE, envp);
ebeff055 David Kershner 2016-09-19 1190 }
ebeff055 David Kershner 2016-09-19 1191

:::::: The code at line 1189 was first introduced by commit
:::::: ae0fa822d7d455ba8974fb5fa1d437bfd1811a7a staging: unisys: visorbus: add error handling for parahotplug_request_kickoff

:::::: TO: David Kershner <[email protected]>
:::::: CC: Greg Kroah-Hartman <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (6.45 kB)
.config.gz (24.23 kB)
Download all attachments