Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer returned by usb_ep_get()

[Liam Merwick]

On 31/01/2019 08:03, Gerd Hoffmann wrote:
> On Wed, Jan 30, 2019 at 02:37:02PM +0000, Liam Merwick wrote:
> > From: Liam Merwick <address@hidden>
> >
> > usb_ep_get() can return a Null pointer in the (albeit unlikely) case
> > that a NULL USBDevice is passed in via the 'dev' parameter.
> That should never ever happen.
>
> > Reported by the Parfait static code analysis tool
> Try add "assert(dev != NULL)" to usb_ep_get() instead of sprinkling
> pointless checks all over the place.
Adding "assert(dev != NULL)" to usb_ep_get() isn't sufficient for that 
tool unless the 'if (dev== NULL)' check is removed which seems a 
backwards step even if that NULL USBDevice case is impossible.

Adding an assert like below in 7 places in hw/usb/core.c that call 
usb_ep_get() would resolve it but would that  pass coding conventions 
(checkpatch.pl seems OK with it)?

 uint8_t usb_ep_get_type(USBDevice *dev, int pid, int ep)
 {
+    assert(dev);
     struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
     return uep->type;
 }

(the other option below it seems like too much code churn).


 uint8_t usb_ep_get_type(USBDevice *dev, int pid, int ep)
 {
-    struct USBEndpoint *uep = usb_ep_get(dev, pid, ep);
+    struct USBEndpoint *uep;
+    assert(dev);
+    uep = usb_ep_get(dev, pid, ep);
     return uep->type;
 }


So that's kinda a long way of saying I'll drop this patch unless someone 
thinks it still adds a benefit and will send a v2 with a modified Patch1 
and a patch that adds two asserts to hw/usb/hcd-xhci.c

Regards,

Liam