Hi, On 12/21/2016 03:38 PM, Shuah Khan wrote: > Hi Sudip, > > On Wed, Dec 21, 2016 at 6:33 AM, Sudip Mukherjee > <sudipm.mukherjee@xxxxxxxxx> wrote: >> On Tue, Dec 20, 2016 at 07:31:44AM -0700, Shuah Khan wrote: >>> On 12/18/2016 03:44 PM, Sudip Mukherjee wrote: >>>> to_vep() is doing a container_of() on _ep. It is better to do the NULL >>>> check first and then use it. >>>> >>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@xxxxxxxxxxxxxxx> >>>> --- >>>> drivers/usb/usbip/vudc_dev.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c >>>> index 968471b..32ea604 100644 >>>> --- a/drivers/usb/usbip/vudc_dev.c >>>> +++ b/drivers/usb/usbip/vudc_dev.c >>>> @@ -388,10 +388,10 @@ static int vep_dequeue(struct usb_ep *_ep, struct usb_request *_req) >>>> unsigned long flags; >>>> int ret = 0; >>>> >>>> - ep = to_vep(_ep); >>>> if (!_ep) >>>> return -EINVAL; >>> >>> Hmm. Linus's latest checks _ep and _req. Are you sure you are working >>> with the latest tree? >> >> I checked with next-20161221 and its still there. > > This is for vep_dequeue() - Are you sure both linux-next and Linus's tree show > the following: > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/usbip/vudc_dev.c > > static int vep_dequeue(struct usb_ep *_ep, struct usb_request *_req) > { > struct vep *ep; > struct vrequest *req; > struct vudc *udc; > struct vrequest *lst; > unsigned long flags; > int ret = -EINVAL; > > if (!_ep || !_req) > return ret; > > ep = to_vep(_ep); > req = to_vrequest(_req); > udc = req->udc; > > if (!udc->driver) > return -ESHUTDOWN; > > There are other routines in this file that don't check null. I am confused. > generally this file contains implementation of gadget/endpoint callbacks. Those methods will be called by udc-core with values passed from USB gadget (usually directly from function). I check other udc in kernel and there is no agreement if we should check in those callbacks if our params are NULL or not. I have also run through udc-core implementation and generally it doesn't check if params are NULL or not and just dereference some of them and pass them to our callbacks. I think that the best option is just to ask Felipe (USB gadget maintainer) if we should check this or not. @Felipe Should each UDC check if values passed to gadget/endpoint callbacks is not NULL or just simply dereference them? Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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
