Re: [PATCH] net: mdio: Use helper function IS_ERR_OR_NULL()
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
Date: 2023-09-07 16:23:50
Also in:
lkml
On Thu, Sep 07, 2023 at 03:17:05AM -0400, Bo Liu wrote:
Use IS_ERR_OR_NULL() to detect an error pointer or a null pointer open-coding to simplify the code. Signed-off-by: Bo Liu <redacted>
Please do a more thorough review of the code before proposing changes to discover what the correct solution should be.
quoted hunk
diff --git a/drivers/net/mdio/mdio-xgene.c b/drivers/net/mdio/mdio-xgene.c index 1190a793555a..1518abfc3e22 100644 --- a/drivers/net/mdio/mdio-xgene.c +++ b/drivers/net/mdio/mdio-xgene.c@@ -263,7 +263,7 @@ struct phy_device *xgene_enet_phy_register(struct mii_bus *bus, int phy_addr) struct phy_device *phy_dev; phy_dev = get_phy_device(bus, phy_addr, false); - if (!phy_dev || IS_ERR(phy_dev)) + if (IS_ERR_OR_NULL(phy_dev))
Looking at get_phy_device(), the returns from this function are either:
if (r)
return ERR_PTR(r);
which can't return NULL, or the return value from phy_device_create().
Looking at phy_device_create(), the returns from this function are:
if (!dev)
return ERR_PTR(-ENOMEM);
if (ret) {
...
dev = ERR_PTR(ret);
...
return dev;
so I don't see any of this being able to return NULL either. Therefore,
the code that you are modifying should be:
- if (!phy_dev || IS_ERR(phy_dev))
+ if (IS_ERR(phy_dev))
and the commit description needs to be updated to state that
get_phy_device() does not return NULL.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!