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. 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 |
Attachment:
signature.asc
Description: OpenPGP digital signature