linux-kernel - Re: [PATCH 5/5] usb: add pxa1928 ehci support

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <4475721.5hJuZmGMm7@wuerfel>
Date:	Fri, 15 May 2015 11:32:12 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Alan Stern <stern@...land.harvard.edu>
Cc:	Rob Herring <robh@...nel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Kishon Vijay Abraham I <kishon@...com>,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	linux-usb@...r.kernel.org
Subject: Re: [PATCH 5/5] usb: add pxa1928 ehci support

On Thursday 14 May 2015 11:55:36 Alan Stern wrote:
> 
> > +     hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
> > +     if (IS_ERR_OR_NULL(hcd->phy)) {
> > +             retval = PTR_ERR(hcd->phy);
> > +             if (retval != -EPROBE_DEFER && retval != -ENODEV)
> > +                     dev_err(&pdev->dev, "failed to get the phy\n");
> > +             else
> > +                     return -EPROBE_DEFER;
> > +             goto err_put_hcd;
> > +     }
> 
> Please straighten out this convoluted logic.  It should go like this:
> 
>         hcd->phy = devm_of_phy_get(&pdev->dev, pdev->dev.of_node, NULL);
>         if (IS_ERR_OR_NULL(hcd->phy)) {
>                 retval = PTR_ERR(hcd->phy);
>                 if (retval == -EPROBE_DEFER || retval == -ENODEV)
>                         return -EPROBE_DEFER;
>                 dev_err(&pdev->dev, "failed to get the phy\n");
>                 goto err_put_hcd;
>         }
> 

IS_ERR_OR_NULL is almost always wrong. Kernel interfaces that can return
an error pointer should never return a NULL pointer, and I'm pretty sure
that is also the case for the phy subsystem (or else it should be fixed).

In some cases, we have interfaces that return NULL pointers or error pointers,
but those are designed to treat NULL as success.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/