On Mon, 6 Aug 2012, Simon Horman wrote:
> On Sun, Aug 05, 2012 at 06:51:34PM -0700, kuninori.morimoto.gx@xxxxxxxxxxx wrote:
> > We should avoid using BUG_ON() in drivers.
> > This patch switch to use WARN_ON() from BUG_ON(),
> > and avoid NULL pointer access by new macro.
> >
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> > ---
> > v2 -> v3
> >
> > - BUG_ON -> WARN_ON
> >
> > drivers/usb/host/ehci-platform.c | 18 ++++++++++--------
> > 1 files changed, 10 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> > index 4b1d896..db27dfe 100644
> > --- a/drivers/usb/host/ehci-platform.c
> > +++ b/drivers/usb/host/ehci-platform.c
> > @@ -21,6 +21,8 @@
> > #include <linux/platform_device.h>
> > #include <linux/usb/ehci_pdriver.h>
> >
> > +#define ehci_pdata_get(pdata, x) ((pdata) ? (pdata)->x : 0)
> > +
>
> FWIW, I think that an inline function would be a slight improvement over
> a macro here. Likewise for the 2nd patch of this series.
No; we should not have either an inline function or a macro. This
computation isn't necessary at all.
If you're worried about the possibility that pdata might be NULL,
just return -ENODEV from ehci_platform_probe() if it is:
int irq;
int err = -ENOMEM;
- BUG_ON(!dev->dev.platform_data);
+ if (!dev->dev.platform_data) {
+ WARN_ON(1);
+ return -ENODEV;
+ }
if (usb_disabled())
return -ENODEV;
But even that may be unnecessary. Simply removing the BUG_ON will
accomplish pretty much the same thing; people will realize something is
wrong pretty quickly when the computer gets an access violation from
trying to dereference a null pointer.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
