| Message ID | 20211028072306.1351-1-yajun.deng@linux.dev (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] batman-adv: Fix the wrong definition | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 2 maintainers not CCed: davem@davemloft.net kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Fixes tag looks correct |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
On Thursday, 28 October 2021 09:23:06 CEST Yajun Deng wrote: > There are three variables that are required at most, > no need to define four variables. NAck. This is absolutely wrong - the last one is the "STOP" info. With your patch, it would sometimes (action != BATADV_UEV_DEL) not have the stop NULL. See also the second parameter in this for loop on line https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/lib/kobject_uevent.c?id=1fc596a56b334f4d593a2b49e5ff55af6aaa0816#n548 We can discuss that this can be written in a better way. See https://patchwork.open-mesh.org/project/b.a.t.m.a.n./patch/1403982781.9064.33.camel@joe-AO725/ (also the remark from Antonio). > > Fixes: 0fa4c30d710d ("batman-adv: Make sysfs support optional") Even this Fixes would be wrong. The code is there since commit c6bda689c2c9 ("batman-adv: add wrapper function to throw uevent in userspace"). But even then, this would not fix anything but just be a cleanup. > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > --- > net/batman-adv/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c > index 3ddd66e4c29e..758035b3796d 100644 > --- a/net/batman-adv/main.c > +++ b/net/batman-adv/main.c > @@ -656,7 +656,7 @@ int batadv_throw_uevent(struct batadv_priv *bat_priv, enum batadv_uev_type type, > { > int ret = -ENOMEM; > struct kobject *bat_kobj; > - char *uevent_env[4] = { NULL, NULL, NULL, NULL }; > + char *uevent_env[3] = {}; > > bat_kobj = &bat_priv->soft_iface->dev.kobj; > >
Hi, On 28/10/2021 09:23, Yajun Deng wrote: > There are three variables that are required at most, > no need to define four variables. > > Fixes: 0fa4c30d710d ("batman-adv: Make sysfs support optional") > Signed-off-by: Yajun Deng <yajun.deng@linux.dev> NAK. kobject_uevent_env() does not know how many items are stored in the array and thus requires it to be NULL terminated. Please check the following for reference: https://elixir.bootlin.com/linux/v5.15-rc6/source/lib/kobject_uevent.c#L548 OTOH I guess we could still use '{}' for the initialization. Regards,
October 28, 2021 3:35 PM, "Antonio Quartulli" <a@unstable.cc> 写到: > Hi, > > On 28/10/2021 09:23, Yajun Deng wrote: > >> There are three variables that are required at most, >> no need to define four variables. >> >> Fixes: 0fa4c30d710d ("batman-adv: Make sysfs support optional") >> Signed-off-by: Yajun Deng <yajun.deng@linux.dev> > > NAK. > > kobject_uevent_env() does not know how many items are stored in the > array and thus requires it to be NULL terminated. > > Please check the following for reference: > > https://elixir.bootlin.com/linux/v5.15-rc6/source/lib/kobject_uevent.c#L548 > Oh, I didn't notice there. > OTOH I guess we could still use '{}' for the initialization. > > Regards, > > -- > Antonio Quartulli
diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c index 3ddd66e4c29e..758035b3796d 100644 --- a/net/batman-adv/main.c +++ b/net/batman-adv/main.c @@ -656,7 +656,7 @@ int batadv_throw_uevent(struct batadv_priv *bat_priv, enum batadv_uev_type type, { int ret = -ENOMEM; struct kobject *bat_kobj; - char *uevent_env[4] = { NULL, NULL, NULL, NULL }; + char *uevent_env[3] = {}; bat_kobj = &bat_priv->soft_iface->dev.kobj;
There are three variables that are required at most, no need to define four variables. Fixes: 0fa4c30d710d ("batman-adv: Make sysfs support optional") Signed-off-by: Yajun Deng <yajun.deng@linux.dev> --- net/batman-adv/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)