Hi Kieran,
Thank you for the patch.
On Friday, 9 November 2018 19:05:29 EET Kieran Bingham wrote:
> The streaming object is a key part of handling the UVC device. Although
> not critical, we are currently missing a call to destroy the mutex on
> clean up paths, and we are due to extend the objects complexity in the
> near future.
>
> Facilitate easy management of a stream object by creating a pair of
> functions to handle creating and destroying the allocation. The new
> uvc_stream_delete() function also performs the missing mutex_destroy()
> operation.
>
> Previously a failed streaming object allocation would cause
> uvc_parse_streaming() to return -EINVAL, which is inappropriate. If the
> constructor failes, we will instead return -ENOMEM.
>
> While we're here, fix the trivial spelling error in the function banner
> of uvc_delete().
>
> Signed-off-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> ---
> drivers/media/usb/uvc/uvc_driver.c | 54 +++++++++++++++++++++----------
> 1 file changed, 38 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index 67bd58c6f397..afb44d1c9d04
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -396,6 +396,39 @@ static struct uvc_streaming *uvc_stream_by_id(struct
> uvc_device *dev, int id) }
>
> /* ------------------------------------------------------------------------
> + * Streaming Object Management
> + */
> +
> +static void uvc_stream_delete(struct uvc_streaming *stream)
> +{
> + mutex_destroy(&stream->mutex);
> +
> + usb_put_intf(stream->intf);
> +
> + kfree(stream->format);
> + kfree(stream->header.bmaControls);
> + kfree(stream);
> +}
> +
> +static struct uvc_streaming *uvc_stream_new(struct uvc_device *dev,
> + struct usb_interface *intf)
> +{
> + struct uvc_streaming *stream;
> +
> + stream = kzalloc(sizeof(*stream), GFP_KERNEL);
> + if (stream == NULL)
> + return NULL;
> +
> + mutex_init(&stream->mutex);
> +
> + stream->dev = dev;
> + stream->intf = usb_get_intf(intf);
> + stream->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +
> + return stream;
> +}
> +
> +/* ------------------------------------------------------------------------
> * Descriptors parsing
> */
>
> @@ -687,17 +720,12 @@ static int uvc_parse_streaming(struct uvc_device *dev,
> return -EINVAL;
> }
>
> - streaming = kzalloc(sizeof(*streaming), GFP_KERNEL);
> + streaming = uvc_stream_new(dev, intf);
> if (streaming == NULL) {
> usb_driver_release_interface(&uvc_driver.driver, intf);
> - return -EINVAL;
> + return -ENOMEM;
> }
>
> - mutex_init(&streaming->mutex);
> - streaming->dev = dev;
> - streaming->intf = usb_get_intf(intf);
> - streaming->intfnum = intf->cur_altsetting->desc.bInterfaceNumber;
> -
> /* The Pico iMage webcam has its class-specific interface descriptors
> * after the endpoint descriptors.
> */
> @@ -904,10 +932,7 @@ static int uvc_parse_streaming(struct uvc_device *dev,
>
> error:
> usb_driver_release_interface(&uvc_driver.driver, intf);
> - usb_put_intf(intf);
> - kfree(streaming->format);
> - kfree(streaming->header.bmaControls);
> - kfree(streaming);
> + uvc_stream_delete(streaming);
> return ret;
> }
>
> @@ -1815,7 +1840,7 @@ static int uvc_scan_device(struct uvc_device *dev)
> * is released.
> *
> * As this function is called after or during disconnect(), all URBs have
> - * already been canceled by the USB core. There is no need to kill the
> + * already been cancelled by the USB core. There is no need to kill the
> * interrupt URB manually.
> */
> static void uvc_delete(struct kref *kref)
> @@ -1853,10 +1878,7 @@ static void uvc_delete(struct kref *kref)
> streaming = list_entry(p, struct uvc_streaming, list);
> usb_driver_release_interface(&uvc_driver.driver,
> streaming->intf);
> - usb_put_intf(streaming->intf);
> - kfree(streaming->format);
> - kfree(streaming->header.bmaControls);
> - kfree(streaming);
> + uvc_stream_delete(streaming);
> }
>
> kfree(dev);
--
Regards,
Laurent Pinchart
