[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer retur
From: |
Liam Merwick |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer returned by usb_ep_get() |
Date: |
Mon, 4 Feb 2019 11:50:33 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
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
- Re: [Qemu-devel] [PATCH 2/2] usb: deal with potential Null pointer returned by usb_ep_get(),
Liam Merwick <=