netdev - Re: [PATCH v2 2/3] of_mdio: use IS_ERR_OR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 13 Mar 2016 23:22:54 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	grant.likely@...aro.org, robh+dt@...nel.org,
	devicetree@...r.kernel.org, f.fainelli@...il.com,
	netdev@...r.kernel.org, frowand.list@...il.com
Subject: Re: [PATCH v2 2/3] of_mdio: use IS_ERR_OR_NULL()

On Sunday 13 March 2016 00:34:02 Sergei Shtylyov wrote:
> IS_ERR_OR_NULL() is open coded in of_mdiobus_register_phy(), so just call
> it directly...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
> Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
> 
> ---
> Changes in version 2:
> - removed the of_mdiobus_register_device() hunk;
> - added the "Reviewed-by:" tag.
> 
>  drivers/of/of_mdio.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: net-next/drivers/of/of_mdio.c
> ===================================================================
> --- net-next.orig/drivers/of/of_mdio.c
> +++ net-next/drivers/of/of_mdio.c
> @@ -56,7 +56,7 @@ static int of_mdiobus_register_phy(struc
>                 phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
>         else
>                 phy = get_phy_device(mdio, addr, is_c45);
> -       if (!phy || IS_ERR(phy))
> +       if (IS_ERR_OR_NULL(phy))
>                 return 1;
>  
>         rc = irq_of_parse_and_map(child, 0);
> 
> 

IS_ERR_OR_NULL() usually indicates that the code is wrong, or that
an API has been misdesigned.

Can you clarify in the changelog which one it is, or (better) change
it so that 'phy' in this function is either NULL or IS_ERR() in case
of an error but not both?

I think moving the 'if (problem) return 1' into the two if/else
is a correct transformation that also makes it very clear what
is going on here.

	Arnd