2023-01-12 16:17:01

by Piergiorgio Beruto

[permalink] [raw]
Subject: [PATCH v2 net-next 1/1] plca.c: fix obvious mistake in checking retval

This patch addresses a wrong fix that was done during the review
process. The intention was to substitute "if(ret < 0)" with
"if(ret)". Unfortunately, in this specific file the intended fix did not
meet the code. After additional review, it seems like if(ret < 0) was
actually the right thing to do. So this patch reverts those changes.

Fixes: 8580e16c28f3 ("net/ethtool: add netlink interface for the PLCA RS")
Signed-off-by: Piergiorgio Beruto <[email protected]>
Reviewed-by: Andrew Lunn <[email protected]>
---
net/ethtool/plca.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/ethtool/plca.c b/net/ethtool/plca.c
index d9bb13ffc654..be7404dc9ef2 100644
--- a/net/ethtool/plca.c
+++ b/net/ethtool/plca.c
@@ -61,7 +61,7 @@ static int plca_get_cfg_prepare_data(const struct ethnl_req_info *req_base,
}

ret = ethnl_ops_begin(dev);
- if (!ret)
+ if (ret < 0)
goto out;

memset(&data->plca_cfg, 0xff,
@@ -151,7 +151,7 @@ int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
tb[ETHTOOL_A_PLCA_HEADER],
genl_info_net(info), info->extack,
true);
- if (!ret)
+ if (ret < 0)
return ret;

dev = req_info.dev;
@@ -171,7 +171,7 @@ int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
}

ret = ethnl_ops_begin(dev);
- if (!ret)
+ if (ret < 0)
goto out_rtnl;

memset(&plca_cfg, 0xff, sizeof(plca_cfg));
@@ -189,7 +189,7 @@ int ethnl_set_plca_cfg(struct sk_buff *skb, struct genl_info *info)
goto out_ops;

ret = ops->set_plca_cfg(dev->phydev, &plca_cfg, info->extack);
- if (!ret)
+ if (ret < 0)
goto out_ops;

ethtool_notify(dev, ETHTOOL_MSG_PLCA_NTF, NULL);
@@ -233,7 +233,7 @@ static int plca_get_status_prepare_data(const struct ethnl_req_info *req_base,
}

ret = ethnl_ops_begin(dev);
- if (!ret)
+ if (ret < 0)
goto out;

memset(&data->plca_st, 0xff,
--
2.37.4


2023-01-13 08:27:03

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/1] plca.c: fix obvious mistake in checking retval

On Thu, Jan 12, 2023 at 04:56:11PM +0100, Piergiorgio Beruto wrote:
> This patch addresses a wrong fix that was done during the review
> process. The intention was to substitute "if(ret < 0)" with
> "if(ret)". Unfortunately, in this specific file the intended fix did not
> meet the code. After additional review, it seems like if(ret < 0) was
> actually the right thing to do. So this patch reverts those changes.

Try to reword the patch description without writing "This patch does foo"
(prefer imperative mood over descriptive one).

Thanks.

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (619.00 B)
signature.asc (235.00 B)
Download all attachments

2023-01-13 13:53:23

by Piergiorgio Beruto

[permalink] [raw]
Subject: Re: [PATCH v2 net-next 1/1] plca.c: fix obvious mistake in checking retval

On Fri, Jan 13, 2023 at 03:23:17PM +0700, Bagas Sanjaya wrote:
> On Thu, Jan 12, 2023 at 04:56:11PM +0100, Piergiorgio Beruto wrote:
> > This patch addresses a wrong fix that was done during the review
> > process. The intention was to substitute "if(ret < 0)" with
> > "if(ret)". Unfortunately, in this specific file the intended fix did not
> > meet the code. After additional review, it seems like if(ret < 0) was
> > actually the right thing to do. So this patch reverts those changes.
>
> Try to reword the patch description without writing "This patch does foo"
> (prefer imperative mood over descriptive one).
>
> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara
Fixed. Thanks.