[PATCH net-next] devlink: remove some unnecessary code

[PATCH net-next] devlink: remove some unnecessary code

[Dan Carpenter]

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(-)

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;
 
-- 
2.35.1

Re: [PATCH net-next] devlink: remove some unnecessary code

[Jiri Pirko]

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>

Re: [PATCH net-next] devlink: remove some unnecessary code

[Jakub Kicinski]

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>

Re: [PATCH net-next] devlink: remove some unnecessary code

[Ido Schimmel]

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>

Re: [PATCH net-next] devlink: remove some unnecessary code

[patchwork-bot+netdevbpf]

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html