diff mbox series

[v6,06/10] media: uvcvideo: Abstract streaming object lifetime

Message ID 207851fbe7c980dee983f5c3587f911457e37f34.1541782862.git-series.kieran.bingham@ideasonboard.com (mailing list archive)
State New, archived
Headers show
Series Asynchronous UVC | expand

Commit Message

Kieran Bingham Nov. 9, 2018, 5:05 p.m. UTC 
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.bingham@ideasonboard.com>
---
 drivers/media/usb/uvc/uvc_driver.c | 54 +++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 16 deletions(-)

Comments

Laurent Pinchart Dec. 4, 2018, 12:51 p.m. UTC | #1 
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.bingham@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@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);
diff mbox series

Patch

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);