On Fri, 3 Jun 2016, Krzysztof Opasiak wrote: > On 06/02/2016 03:22 PM, Sudip Mukherjee wrote: > > We have been dereferencing udc before checking it. Lets use it after it > > has been checked. > > > > To be honest I have mixed feelings about this patch. > > On one hand it prevents us from dereferencing potential NULL ptr what is > generally good. But on the other hand it seems to be a little bit > pointless overhead. This function is called only in one place, it's > internal function of vudc driver and in addition generally it is > currently impossible that this function will get NULL ptr as parameter > as it's value is taken from container_of(). Not to mention that if this > is NULL or garbage we will end up in NULL ptr dereference much earlier > before calling this function. > > So if there is something that you would like to fix with this patch and > you have a real problem with this function could you please provide us > some more details (for example stack trace)? If this patch is just to > prevent us from something that will never happen then I would rather to > not submit this. In my opinion if we get a NULL in this function this > means that we have some serious problem in UDC core and this check will > just mask this error. Normally in this situation, somebody would write a patch that removes these lines from get_gadget_descs(): if (!udc || !udc->driver || !udc->pullup) return -EINVAL; Or maybe (I'm not familiar with the driver so I don't know the best approach) changes the test to: if (!udc->driver || !udc->pullup) After all, there's no reason to check !udc if it is known beforehand that udc will never be NULL. 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
