On Tue, Nov 20, 2012 at 02:43:40PM +0100, Marc Kleine-Budde wrote: > On 11/20/2012 02:40 PM, Peter Chen wrote: > > On Tue, Nov 20, 2012 at 12:41:03PM +0100, Marc Kleine-Budde wrote: > >> On 11/20/2012 02:33 AM, Peter Chen wrote: > >>> On Mon, Nov 19, 2012 at 05:05:16PM +0100, Marc Kleine-Budde wrote: > >>>>> +int ci_hdrc_otg_init(struct ci13xxx *ci) > >>>>> +{ > >>>>> + /* Useless at current */ > >>>>> + ci->otg.set_peripheral = ci_otg_set_peripheral; > >>>>> + ci->otg.set_host = ci_otg_set_host; > >>>>> + if (!IS_ERR_OR_NULL(ci->transceiver)) > >>>> ^^^^^^^^^^^^^^ > >>>> > >>>> Is considered harmful by some people. Can we test against NULL here? > >>> > >>> Any stories that it is harmful? > >> > >> Your use-case isn't but there others are: > >> http://www.mail-archive.com/linux-omap@xxxxxxxxxxxxxxx/msg78030.html > > > > Thanks, marc. It said below code is error if the foo is really null. > > if (IS_ERR_OR_NULL(foo)) > > return PTR_ERR(foo); > > Yes. So my next comment is nitpicking: :) > > > For this case, the ci->transceiver may be null or errno pointer, it > > needs IS_ERR_OR_NULL. > > Yes, but you can set ci->transceiver to NULL in case of an error when > assigning it, to avoid IS_ERR_OR_NULL here. OK, as the chipidea/udc.c also uses this macro. I will submit a patch to change them together later, not sure if it is necessary for everyone. > > Marc > > -- > Pengutronix e.K. | Marc Kleine-Budde | > Industrial Linux Solutions | Phone: +49-231-2826-924 | > Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | > Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | > -- Best Regards, Peter Chen -- 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