| Message ID | Y8EJz8oxpMhfiPUb@kili (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 501543b4fff0ff70bde28a829eb8835081ccef2f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] devlink: remove some unnecessary code | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 2 this patch: 2 |
| netdev/cc_maintainers | success | CCed 6 of 6 maintainers |
| netdev/build_clang | success | Errors and warnings before: 1 this patch: 1 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 2 this patch: 2 |
| netdev/checkpatch | warning | WARNING: line length of 88 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
Fri, Jan 13, 2023 at 08:35:43AM CET, error27@gmail.com wrote: >This code checks if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) twice. Once >at the start of the function and then a couple lines later. Delete the >second check since that one must be true. > >Because the second condition is always true, it means the: > > policer_item = group_item->policer_item; > >assignment is immediately over-written. Delete that as well. > >Signed-off-by: Dan Carpenter <error27@gmail.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
On Fri, 13 Jan 2023 10:35:43 +0300 Dan Carpenter wrote: > This code checks if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) twice. Once > at the start of the function and then a couple lines later. Delete the > second check since that one must be true. > > Because the second condition is always true, it means the: > > policer_item = group_item->policer_item; > > assignment is immediately over-written. Delete that as well. > > Signed-off-by: Dan Carpenter <error27@gmail.com> Acked-by: Jakub Kicinski <kuba@kernel.org>
On Fri, Jan 13, 2023 at 10:35:43AM +0300, Dan Carpenter wrote: > This code checks if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) twice. Once > at the start of the function and then a couple lines later. Delete the > second check since that one must be true. > > Because the second condition is always true, it means the: > > policer_item = group_item->policer_item; > > assignment is immediately over-written. Delete that as well. > > Signed-off-by: Dan Carpenter <error27@gmail.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Hello: This patch was applied to netdev/net-next.git (master) by Paolo Abeni <pabeni@redhat.com>: On Fri, 13 Jan 2023 10:35:43 +0300 you wrote: > This code checks if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) twice. Once > at the start of the function and then a couple lines later. Delete the > second check since that one must be true. > > Because the second condition is always true, it means the: > > policer_item = group_item->policer_item; > > [...] Here is the summary with links: - [net-next] devlink: remove some unnecessary code https://git.kernel.org/netdev/net-next/c/501543b4fff0 You are awesome, thank you!
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 1e23b2da78cc..bf5e0b1c0422 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -8719,6 +8719,7 @@ static int devlink_trap_group_set(struct devlink *devlink, struct netlink_ext_ack *extack = info->extack; const struct devlink_trap_policer *policer; struct nlattr **attrs = info->attrs; + u32 policer_id; int err; if (!attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) @@ -8727,17 +8728,11 @@ static int devlink_trap_group_set(struct devlink *devlink, if (!devlink->ops->trap_group_set) return -EOPNOTSUPP; - policer_item = group_item->policer_item; - if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) { - u32 policer_id; - - policer_id = nla_get_u32(attrs[DEVLINK_ATTR_TRAP_POLICER_ID]); - policer_item = devlink_trap_policer_item_lookup(devlink, - policer_id); - if (policer_id && !policer_item) { - NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer"); - return -ENOENT; - } + policer_id = nla_get_u32(attrs[DEVLINK_ATTR_TRAP_POLICER_ID]); + policer_item = devlink_trap_policer_item_lookup(devlink, policer_id); + if (policer_id && !policer_item) { + NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer"); + return -ENOENT; } policer = policer_item ? policer_item->policer : NULL;
This code checks if (attrs[DEVLINK_ATTR_TRAP_POLICER_ID]) twice. Once at the start of the function and then a couple lines later. Delete the second check since that one must be true. Because the second condition is always true, it means the: policer_item = group_item->policer_item; assignment is immediately over-written. Delete that as well. Signed-off-by: Dan Carpenter <error27@gmail.com> --- This is from static analysis and not tested. It's possible (although unlikely) that the static checker found buggy code instead of merely a bit of dead code. Please review this one carefully. net/devlink/leftover.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-)