* [PATCH] staging: unisys: visorbus: Declared char * array as static const
@ 2017-09-09 7:00 Harsha Sharma
2017-09-09 7:14 ` Greg KH
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Harsha Sharma @ 2017-09-09 7:00 UTC (permalink / raw)
To: gregkh; +Cc: devel, linux-kernel, outreachy-kernel, Harsha Sharma
State explicitly that individual entries in array will not change.
Signed-off-by: Harsha Sharma <harshasharmaiitr@gmail.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 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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const
2017-09-09 7:00 [PATCH] staging: unisys: visorbus: Declared char * array as static const Harsha Sharma
@ 2017-09-09 7:14 ` Greg KH
2017-09-09 8:36 ` [Outreachy kernel] " Julia Lawall
2017-09-11 9:30 ` kbuild test robot
2017-09-12 0:29 ` kbuild test robot
2 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2017-09-09 7:14 UTC (permalink / raw)
To: Harsha Sharma; +Cc: devel, linux-kernel, outreachy-kernel
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 <harshasharmaiitr@gmail.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 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const
2017-09-09 7:14 ` Greg KH
@ 2017-09-09 8:36 ` Julia Lawall
0 siblings, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2017-09-09 8:36 UTC (permalink / raw)
To: Greg KH; +Cc: Harsha Sharma, devel, linux-kernel, outreachy-kernel
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 <harshasharmaiitr@gmail.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 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 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/20170909071451.GA27010%40kroah.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const
2017-09-09 7:00 [PATCH] staging: unisys: visorbus: Declared char * array as static const Harsha Sharma
2017-09-09 7:14 ` Greg KH
@ 2017-09-11 9:30 ` kbuild test robot
2017-09-12 0:29 ` kbuild test robot
2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2017-09-11 9:30 UTC (permalink / raw)
To: Harsha Sharma
Cc: kbuild-all, gregkh, devel, outreachy-kernel, Harsha Sharma,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 7329 bytes --]
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 <david.kershner@unisys.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 60296 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] staging: unisys: visorbus: Declared char * array as static const
2017-09-09 7:00 [PATCH] staging: unisys: visorbus: Declared char * array as static const Harsha Sharma
2017-09-09 7:14 ` Greg KH
2017-09-11 9:30 ` kbuild test robot
@ 2017-09-12 0:29 ` kbuild test robot
2 siblings, 0 replies; 5+ messages in thread
From: kbuild test robot @ 2017-09-12 0:29 UTC (permalink / raw)
To: Harsha Sharma
Cc: kbuild-all, gregkh, devel, outreachy-kernel, Harsha Sharma,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 6608 bytes --]
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 <david.kershner@unisys.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24814 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-12 0:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-09 7:00 [PATCH] staging: unisys: visorbus: Declared char * array as static const Harsha Sharma
2017-09-09 7:14 ` Greg KH
2017-09-09 8:36 ` [Outreachy kernel] " Julia Lawall
2017-09-11 9:30 ` kbuild test robot
2017-09-12 0:29 ` kbuild test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox