On 08/06/2021 19:11, Oleksij Rempel wrote: > On Tue, Jun 08, 2021 at 04:22:49PM +0100, Colin King wrote: >> From: Colin Ian King <colin.king@xxxxxxxxxxxxx> >> >> The comparison of the u16 priv->phy_addr < 0 is always false because >> phy_addr is unsigned. Fix this by assigning the return from the call >> to function asix_read_phy_addr to int ret and using this for the >> less than zero error check comparison. >> >> Addresses-Coverity: ("Unsigned compared against 0") >> Fixes: 7e88b11a862a ("net: usb: asix: refactor asix_read_phy_addr() and handle errors on return") > > Here is wrong Fixes tag. This assignment was bogus before this patch. I'm not sure that's correct, that commit has the following change in it: diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c index b404c9462dce..c8ca5187eece 100644 --- a/drivers/net/usb/ax88172a.c +++ b/drivers/net/usb/ax88172a.c @@ -220,6 +220,11 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf) } priv->phy_addr = asix_read_phy_addr(dev, priv->use_embdphy); + if (priv->phy_addr < 0) { + ret = priv->phy_addr; + goto free; + } + > >> Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx> >> --- >> drivers/net/usb/ax88172a.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/usb/ax88172a.c b/drivers/net/usb/ax88172a.c >> index 2e2081346740..e24773bb9398 100644 >> --- a/drivers/net/usb/ax88172a.c >> +++ b/drivers/net/usb/ax88172a.c >> @@ -205,7 +205,8 @@ static int ax88172a_bind(struct usbnet *dev, struct usb_interface *intf) >> goto free; >> } >> >> - priv->phy_addr = asix_read_phy_addr(dev, priv->use_embdphy); >> + ret = asix_read_phy_addr(dev, priv->use_embdphy); >> + priv->phy_addr = ret; > > Ah.. it is same bug in different color :) > You probably wonted to do: > if (ret < 0) > goto free; > > priv->phy_addr = ret; Doh, brain failure of mine. I'll send a V2 later today. > >> if (priv->phy_addr < 0) { >> ret = priv->phy_addr; >> goto free; >> -- >> 2.31.1 >> >> >