[PATCH 0/4] HDPVR series of patches to replace Apr 22 patch

[PATCH 0/4] HDPVR series of patches to replace Apr 22 patch

[Leonid Kegulskiy]

Hi Hans,

This series of patches replace the previous patch sent on Apr 22:
    [PATCH] [media] hdpvr_ error handling and alloc abuse cleanup.

Thank you,
-Leo.

Leonid Kegulskiy (4):
  [media] hdpvr: Removed unnecessary get_video_info() call from
    hdpvr_device_init()
  [media] hdpvr: Removed unnecessary use of kzalloc() in
    get_video_info()
  [media] hdpvr: Added some error handling in hdpvr_start_streaming()
  [media] hdpvr: Cleaned up error handling

 drivers/media/usb/hdpvr/hdpvr-control.c |   20 ++-------
 drivers/media/usb/hdpvr/hdpvr-core.c    |    8 ----
 drivers/media/usb/hdpvr/hdpvr-video.c   |   70 +++++++++++++++++--------------
 drivers/media/usb/hdpvr/hdpvr.h         |    2 +-
 4 files changed, 43 insertions(+), 57 deletions(-)

-- 
1.7.10.4

[PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init()

[Leonid Kegulskiy]

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-core.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c
index 8247c19..cb69405 100644
--- a/drivers/media/usb/hdpvr/hdpvr-core.c
+++ b/drivers/media/usb/hdpvr/hdpvr-core.c
@@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
 {
 	int ret;
 	u8 *buf;
-	struct hdpvr_video_info *vidinf;
 
 	if (device_authorization(dev))
 		return -EACCES;
@@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
 		 "control request returned %d\n", ret);
 	mutex_unlock(&dev->usbc_mutex);
 
-	vidinf = get_video_info(dev);
-	if (!vidinf)
-		v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
-			"no valid video signal or device init failed\n");
-	else
-		kfree(vidinf);
-
 	/* enable fan and bling leds */
 	mutex_lock(&dev->usbc_mutex);
 	buf[0] = 0x1;
-- 
1.7.10.4

[PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info()

[Leonid Kegulskiy]

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-control.c |   21 ++++--------
 drivers/media/usb/hdpvr/hdpvr-video.c   |   54 +++++++++++++++----------------
 drivers/media/usb/hdpvr/hdpvr.h         |    2 +-
 3 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
index ae8f229..5265b75 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf)
 	return ret < 0 ? ret : 0;
 }
 
-struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
+int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
 {
-	struct hdpvr_video_info *vidinf = NULL;
-#ifdef HDPVR_DEBUG
-	char print_buf[15];
-#endif
 	int ret;
 
-	vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL);
-	if (!vidinf) {
-		v4l2_err(&dev->v4l2_dev, "out of memory\n");
-		goto err;
-	}
-
 	mutex_lock(&dev->usbc_mutex);
 	ret = usb_control_msg(dev->udev,
 			      usb_rcvctrlpipe(dev->udev, 0),
@@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
 
 #ifdef HDPVR_DEBUG
 	if (hdpvr_debug & MSG_INFO) {
+		char print_buf[15];
 		hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf,
 				   sizeof(print_buf), 0);
 		v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
@@ -82,12 +73,12 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
 #endif
 	mutex_unlock(&dev->usbc_mutex);
 
+	/* preserve original behavior - fail if no signal is detected */
 	if (!vidinf->width || !vidinf->height || !vidinf->fps) {
-		kfree(vidinf);
-		vidinf = NULL;
+		ret = -EFAULT;
 	}
-err:
-	return vidinf;
+
+	return ret < 0 ? ret : 0;
 }
 
 int get_input_lines_info(struct hdpvr_device *dev)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 774ba0e..d9eb8e1 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -277,20 +277,19 @@ error:
 static int hdpvr_start_streaming(struct hdpvr_device *dev)
 {
 	int ret;
-	struct hdpvr_video_info *vidinf;
+	struct hdpvr_video_info vidinf;
 
 	if (dev->status == STATUS_STREAMING)
 		return 0;
 	else if (dev->status != STATUS_IDLE)
 		return -EAGAIN;
 
-	vidinf = get_video_info(dev);
+	ret = get_video_info(dev, &vidinf);
 
-	if (vidinf) {
+	if (!ret) {
 		v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
-			 "video signal: %dx%d@%dhz\n", vidinf->width,
-			 vidinf->height, vidinf->fps);
-		kfree(vidinf);
+			 "video signal: %dx%d@%dhz\n", vidinf.width,
+			 vidinf.height, vidinf.fps);
 
 		/* start streaming 2 request */
 		ret = usb_control_msg(dev->udev,
@@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh,
 static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
 {
 	struct hdpvr_device *dev = video_drvdata(file);
-	struct hdpvr_video_info *vid_info;
+	struct hdpvr_video_info vid_info;
 	struct hdpvr_fh *fh = _fh;
+	int ret;
 
 	*a = V4L2_STD_ALL;
 	if (dev->options.video_input == HDPVR_COMPONENT)
 		return fh->legacy_mode ? 0 : -ENODATA;
-	vid_info = get_video_info(dev);
-	if (vid_info == NULL)
+	ret = get_video_info(dev, &vid_info);
+	if (ret)
 		return 0;
-	if (vid_info->width == 720 &&
-	    (vid_info->height == 480 || vid_info->height == 576)) {
-		*a = (vid_info->height == 480) ?
+	if (vid_info.width == 720 &&
+	    (vid_info.height == 480 || vid_info.height == 576)) {
+		*a = (vid_info.height == 480) ?
 			V4L2_STD_525_60 : V4L2_STD_625_50;
 	}
-	kfree(vid_info);
+
 	return 0;
 }
 
@@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 {
 	struct hdpvr_device *dev = video_drvdata(file);
 	struct hdpvr_fh *fh = _fh;
-	struct hdpvr_video_info *vid_info;
+	struct hdpvr_video_info vid_info;
 	bool interlaced;
 	int ret = 0;
 	int i;
@@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 	fh->legacy_mode = false;
 	if (dev->options.video_input)
 		return -ENODATA;
-	vid_info = get_video_info(dev);
-	if (vid_info == NULL)
+	ret = get_video_info(dev, &vid_info);
+	if (ret)
 		return -ENOLCK;
-	interlaced = vid_info->fps <= 30;
+	interlaced = vid_info.fps <= 30;
 	for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) {
 		const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt;
 		unsigned hsize;
@@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 			bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch +
 			bt->height;
 		fps = (unsigned)bt->pixelclock / (hsize * vsize);
-		if (bt->width != vid_info->width ||
-		    bt->height != vid_info->height ||
+		if (bt->width != vid_info.width ||
+		    bt->height != vid_info.height ||
 		    bt->interlaced != interlaced ||
-		    (fps != vid_info->fps && fps + 1 != vid_info->fps))
+		    (fps != vid_info.fps && fps + 1 != vid_info.fps))
 			continue;
 		*timings = hdpvr_dv_timings[i];
 		break;
 	}
 	if (i == ARRAY_SIZE(hdpvr_dv_timings))
 		ret = -ERANGE;
-	kfree(vid_info);
+
 	return ret;
 }
 
@@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
 {
 	struct hdpvr_device *dev = video_drvdata(file);
 	struct hdpvr_fh *fh = _fh;
+	int ret;
 
 	/*
 	 * The original driver would always returns the current detected
@@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
 	 * last set format.
 	 */
 	if (fh->legacy_mode) {
-		struct hdpvr_video_info *vid_info;
+		struct hdpvr_video_info vid_info;
 
-		vid_info = get_video_info(dev);
-		if (!vid_info)
+		ret = get_video_info(dev, &vid_info);
+		if (ret)
 			return -EFAULT;
-		f->fmt.pix.width = vid_info->width;
-		f->fmt.pix.height = vid_info->height;
-		kfree(vid_info);
+		f->fmt.pix.width = vid_info.width;
+		f->fmt.pix.height = vid_info.height;
 	} else {
 		f->fmt.pix.width = dev->width;
 		f->fmt.pix.height = dev->height;
diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h
index 1478f3d..808ea7a 100644
--- a/drivers/media/usb/hdpvr/hdpvr.h
+++ b/drivers/media/usb/hdpvr/hdpvr.h
@@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input,
 int hdpvr_config_call(struct hdpvr_device *dev, uint value,
 		      unsigned char valbuf);
 
-struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev);
+int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info);
 
 /* :0 s b8 81 1800 0003 0003 3 < */
 /* :0 0 3 = 0301ff */
-- 
1.7.10.4

Re: [PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info()

[Hans Verkuil]

Hi Leonid,

I've accepted patch 1 & 3, but this patch needs a bit more work, see below:

On Thu April 25 2013 11:59:55 Leonid Kegulskiy wrote:
> Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
> ---
>  drivers/media/usb/hdpvr/hdpvr-control.c |   21 ++++--------
>  drivers/media/usb/hdpvr/hdpvr-video.c   |   54 +++++++++++++++----------------
>  drivers/media/usb/hdpvr/hdpvr.h         |    2 +-
>  3 files changed, 34 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
> index ae8f229..5265b75 100644
> --- a/drivers/media/usb/hdpvr/hdpvr-control.c
> +++ b/drivers/media/usb/hdpvr/hdpvr-control.c
> @@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf)
>  	return ret < 0 ? ret : 0;
>  }
>  
> -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
> +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
>  {
> -	struct hdpvr_video_info *vidinf = NULL;
> -#ifdef HDPVR_DEBUG
> -	char print_buf[15];
> -#endif
>  	int ret;
>  
> -	vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL);
> -	if (!vidinf) {
> -		v4l2_err(&dev->v4l2_dev, "out of memory\n");
> -		goto err;
> -	}
> -
>  	mutex_lock(&dev->usbc_mutex);
>  	ret = usb_control_msg(dev->udev,
>  			      usb_rcvctrlpipe(dev->udev, 0),
> @@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
>  
>  #ifdef HDPVR_DEBUG
>  	if (hdpvr_debug & MSG_INFO) {
> +		char print_buf[15];
>  		hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf,
>  				   sizeof(print_buf), 0);
>  		v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
> @@ -82,12 +73,12 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
>  #endif
>  	mutex_unlock(&dev->usbc_mutex);
>  
> +	/* preserve original behavior - fail if no signal is detected */
>  	if (!vidinf->width || !vidinf->height || !vidinf->fps) {

This check fails if usb_control_msg returns a value >= 0 but != 5 since in
that case the contents of vidinf is undefined. I would rewrite this so that
any return value != 5 results in an error.

Regards,

	Hans

> -		kfree(vidinf);
> -		vidinf = NULL;
> +		ret = -EFAULT;
>  	}
> -err:
> -	return vidinf;
> +
> +	return ret < 0 ? ret : 0;
>  }
>  
>  int get_input_lines_info(struct hdpvr_device *dev)
> diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
> index 774ba0e..d9eb8e1 100644
> --- a/drivers/media/usb/hdpvr/hdpvr-video.c
> +++ b/drivers/media/usb/hdpvr/hdpvr-video.c
> @@ -277,20 +277,19 @@ error:
>  static int hdpvr_start_streaming(struct hdpvr_device *dev)
>  {
>  	int ret;
> -	struct hdpvr_video_info *vidinf;
> +	struct hdpvr_video_info vidinf;
>  
>  	if (dev->status == STATUS_STREAMING)
>  		return 0;
>  	else if (dev->status != STATUS_IDLE)
>  		return -EAGAIN;
>  
> -	vidinf = get_video_info(dev);
> +	ret = get_video_info(dev, &vidinf);
>  
> -	if (vidinf) {
> +	if (!ret) {
>  		v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
> -			 "video signal: %dx%d@%dhz\n", vidinf->width,
> -			 vidinf->height, vidinf->fps);
> -		kfree(vidinf);
> +			 "video signal: %dx%d@%dhz\n", vidinf.width,
> +			 vidinf.height, vidinf.fps);
>  
>  		/* start streaming 2 request */
>  		ret = usb_control_msg(dev->udev,
> @@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh,
>  static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
>  {
>  	struct hdpvr_device *dev = video_drvdata(file);
> -	struct hdpvr_video_info *vid_info;
> +	struct hdpvr_video_info vid_info;
>  	struct hdpvr_fh *fh = _fh;
> +	int ret;
>  
>  	*a = V4L2_STD_ALL;
>  	if (dev->options.video_input == HDPVR_COMPONENT)
>  		return fh->legacy_mode ? 0 : -ENODATA;
> -	vid_info = get_video_info(dev);
> -	if (vid_info == NULL)
> +	ret = get_video_info(dev, &vid_info);
> +	if (ret)
>  		return 0;
> -	if (vid_info->width == 720 &&
> -	    (vid_info->height == 480 || vid_info->height == 576)) {
> -		*a = (vid_info->height == 480) ?
> +	if (vid_info.width == 720 &&
> +	    (vid_info.height == 480 || vid_info.height == 576)) {
> +		*a = (vid_info.height == 480) ?
>  			V4L2_STD_525_60 : V4L2_STD_625_50;
>  	}
> -	kfree(vid_info);
> +
>  	return 0;
>  }
>  
> @@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
>  {
>  	struct hdpvr_device *dev = video_drvdata(file);
>  	struct hdpvr_fh *fh = _fh;
> -	struct hdpvr_video_info *vid_info;
> +	struct hdpvr_video_info vid_info;
>  	bool interlaced;
>  	int ret = 0;
>  	int i;
> @@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
>  	fh->legacy_mode = false;
>  	if (dev->options.video_input)
>  		return -ENODATA;
> -	vid_info = get_video_info(dev);
> -	if (vid_info == NULL)
> +	ret = get_video_info(dev, &vid_info);
> +	if (ret)
>  		return -ENOLCK;
> -	interlaced = vid_info->fps <= 30;
> +	interlaced = vid_info.fps <= 30;
>  	for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) {
>  		const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt;
>  		unsigned hsize;
> @@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
>  			bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch +
>  			bt->height;
>  		fps = (unsigned)bt->pixelclock / (hsize * vsize);
> -		if (bt->width != vid_info->width ||
> -		    bt->height != vid_info->height ||
> +		if (bt->width != vid_info.width ||
> +		    bt->height != vid_info.height ||
>  		    bt->interlaced != interlaced ||
> -		    (fps != vid_info->fps && fps + 1 != vid_info->fps))
> +		    (fps != vid_info.fps && fps + 1 != vid_info.fps))
>  			continue;
>  		*timings = hdpvr_dv_timings[i];
>  		break;
>  	}
>  	if (i == ARRAY_SIZE(hdpvr_dv_timings))
>  		ret = -ERANGE;
> -	kfree(vid_info);
> +
>  	return ret;
>  }
>  
> @@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
>  {
>  	struct hdpvr_device *dev = video_drvdata(file);
>  	struct hdpvr_fh *fh = _fh;
> +	int ret;
>  
>  	/*
>  	 * The original driver would always returns the current detected
> @@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
>  	 * last set format.
>  	 */
>  	if (fh->legacy_mode) {
> -		struct hdpvr_video_info *vid_info;
> +		struct hdpvr_video_info vid_info;
>  
> -		vid_info = get_video_info(dev);
> -		if (!vid_info)
> +		ret = get_video_info(dev, &vid_info);
> +		if (ret)
>  			return -EFAULT;
> -		f->fmt.pix.width = vid_info->width;
> -		f->fmt.pix.height = vid_info->height;
> -		kfree(vid_info);
> +		f->fmt.pix.width = vid_info.width;
> +		f->fmt.pix.height = vid_info.height;
>  	} else {
>  		f->fmt.pix.width = dev->width;
>  		f->fmt.pix.height = dev->height;
> diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h
> index 1478f3d..808ea7a 100644
> --- a/drivers/media/usb/hdpvr/hdpvr.h
> +++ b/drivers/media/usb/hdpvr/hdpvr.h
> @@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input,
>  int hdpvr_config_call(struct hdpvr_device *dev, uint value,
>  		      unsigned char valbuf);
>  
> -struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev);
> +int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info);
>  
>  /* :0 s b8 81 1800 0003 0003 3 < */
>  /* :0 0 3 = 0301ff */
> 

[PATCH 3/4] [media] hdpvr: Added some error handling in hdpvr_start_streaming()

[Leonid Kegulskiy]

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-video.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index d9eb8e1..2d02b49 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -297,8 +297,12 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev)
 				      0xb8, 0x38, 0x1, 0, NULL, 0, 8000);
 		v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
 			 "encoder start control request returned %d\n", ret);
+		if (ret < 0)
+			return ret;
 
-		hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00);
+		ret = hdpvr_config_call(dev, CTRL_START_STREAMING_VALUE, 0x00);
+		if (ret)
+			return ret;
 
 		dev->status = STATUS_STREAMING;
 
-- 
1.7.10.4

[PATCH 4/4] [media] hdpvr: Cleaned up error handling

[Leonid Kegulskiy]

Changed vidioc_g_fmt_vid_cap() implementation not to return
-EFAULT when video lock is not detected, but return empty
width/height fields (legacy mode only). This new behavior is
supported by MythTV.

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-control.c |    5 -----
 drivers/media/usb/hdpvr/hdpvr-video.c   |   10 +++++++---
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
index 5265b75..16d2d64 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -73,11 +73,6 @@ int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
 #endif
 	mutex_unlock(&dev->usbc_mutex);
 
-	/* preserve original behavior - fail if no signal is detected */
-	if (!vidinf->width || !vidinf->height || !vidinf->fps) {
-		ret = -EFAULT;
-	}
-
 	return ret < 0 ? ret : 0;
 }
 
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 2d02b49..5e8d6c2 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -285,8 +285,10 @@ static int hdpvr_start_streaming(struct hdpvr_device *dev)
 		return -EAGAIN;
 
 	ret = get_video_info(dev, &vidinf);
+	if (ret)		/* device is dead */
+		return ret;	/* let the caller know */
 
-	if (!ret) {
+	if (vidinf.width && vidinf.height) {
 		v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
 			 "video signal: %dx%d@%dhz\n", vidinf.width,
 			 vidinf.height, vidinf.fps);
@@ -618,7 +620,7 @@ static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
 		return fh->legacy_mode ? 0 : -ENODATA;
 	ret = get_video_info(dev, &vid_info);
 	if (ret)
-		return 0;
+		return ret;
 	if (vid_info.width == 720 &&
 	    (vid_info.height == 480 || vid_info.height == 576)) {
 		*a = (vid_info.height == 480) ?
@@ -679,6 +681,8 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 		return -ENODATA;
 	ret = get_video_info(dev, &vid_info);
 	if (ret)
+		return ret;
+	if (vid_info.fps == 0)
 		return -ENOLCK;
 	interlaced = vid_info.fps <= 30;
 	for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) {
@@ -1009,7 +1013,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
 
 		ret = get_video_info(dev, &vid_info);
 		if (ret)
-			return -EFAULT;
+			return ret;
 		f->fmt.pix.width = vid_info.width;
 		f->fmt.pix.height = vid_info.height;
 	} else {
-- 
1.7.10.4

[PATCH 1/4] [media] hdpvr: Removed unnecessary get_video_info() call from hdpvr_device_init()

[Leonid Kegulskiy]

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-core.c |    8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-core.c b/drivers/media/usb/hdpvr/hdpvr-core.c
index 8247c19..cb69405 100644
--- a/drivers/media/usb/hdpvr/hdpvr-core.c
+++ b/drivers/media/usb/hdpvr/hdpvr-core.c
@@ -220,7 +220,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
 {
 	int ret;
 	u8 *buf;
-	struct hdpvr_video_info *vidinf;
 
 	if (device_authorization(dev))
 		return -EACCES;
@@ -242,13 +241,6 @@ static int hdpvr_device_init(struct hdpvr_device *dev)
 		 "control request returned %d\n", ret);
 	mutex_unlock(&dev->usbc_mutex);
 
-	vidinf = get_video_info(dev);
-	if (!vidinf)
-		v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
-			"no valid video signal or device init failed\n");
-	else
-		kfree(vidinf);
-
 	/* enable fan and bling leds */
 	mutex_lock(&dev->usbc_mutex);
 	buf[0] = 0x1;
-- 
1.7.9.5

[PATCH 2/4] [media] hdpvr: Removed unnecessary use of kzalloc() in get_video_info()

[Leonid Kegulskiy]

Signed-off-by: Leonid Kegulskiy <leo@lumanate.com>
---
 drivers/media/usb/hdpvr/hdpvr-control.c |   25 ++++++--------
 drivers/media/usb/hdpvr/hdpvr-video.c   |   54 +++++++++++++++----------------
 drivers/media/usb/hdpvr/hdpvr.h         |    2 +-
 3 files changed, 37 insertions(+), 44 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-control.c b/drivers/media/usb/hdpvr/hdpvr-control.c
index ae8f229..7d1bfb3 100644
--- a/drivers/media/usb/hdpvr/hdpvr-control.c
+++ b/drivers/media/usb/hdpvr/hdpvr-control.c
@@ -45,20 +45,10 @@ int hdpvr_config_call(struct hdpvr_device *dev, uint value, u8 valbuf)
 	return ret < 0 ? ret : 0;
 }
 
-struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
+int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vidinf)
 {
-	struct hdpvr_video_info *vidinf = NULL;
-#ifdef HDPVR_DEBUG
-	char print_buf[15];
-#endif
 	int ret;
 
-	vidinf = kzalloc(sizeof(struct hdpvr_video_info), GFP_KERNEL);
-	if (!vidinf) {
-		v4l2_err(&dev->v4l2_dev, "out of memory\n");
-		goto err;
-	}
-
 	mutex_lock(&dev->usbc_mutex);
 	ret = usb_control_msg(dev->udev,
 			      usb_rcvctrlpipe(dev->udev, 0),
@@ -74,6 +64,7 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
 
 #ifdef HDPVR_DEBUG
 	if (hdpvr_debug & MSG_INFO) {
+		char print_buf[15];
 		hex_dump_to_buffer(dev->usbc_buf, 5, 16, 1, print_buf,
 				   sizeof(print_buf), 0);
 		v4l2_dbg(MSG_INFO, hdpvr_debug, &dev->v4l2_dev,
@@ -82,12 +73,14 @@ struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev)
 #endif
 	mutex_unlock(&dev->usbc_mutex);
 
-	if (!vidinf->width || !vidinf->height || !vidinf->fps) {
-		kfree(vidinf);
-		vidinf = NULL;
+	if ((ret>0 && ret!=5) ||/* fail if unexpected byte count returned */
+	    !vidinf->width ||	/* preserve original behavior -  */
+	    !vidinf->height ||	/* fail if no signal is detected */
+	    !vidinf->fps) {
+		ret = -EFAULT;
 	}
-err:
-	return vidinf;
+
+	return ret < 0 ? ret : 0;
 }
 
 int get_input_lines_info(struct hdpvr_device *dev)
diff --git a/drivers/media/usb/hdpvr/hdpvr-video.c b/drivers/media/usb/hdpvr/hdpvr-video.c
index 774ba0e..d9eb8e1 100644
--- a/drivers/media/usb/hdpvr/hdpvr-video.c
+++ b/drivers/media/usb/hdpvr/hdpvr-video.c
@@ -277,20 +277,19 @@ error:
 static int hdpvr_start_streaming(struct hdpvr_device *dev)
 {
 	int ret;
-	struct hdpvr_video_info *vidinf;
+	struct hdpvr_video_info vidinf;
 
 	if (dev->status == STATUS_STREAMING)
 		return 0;
 	else if (dev->status != STATUS_IDLE)
 		return -EAGAIN;
 
-	vidinf = get_video_info(dev);
+	ret = get_video_info(dev, &vidinf);
 
-	if (vidinf) {
+	if (!ret) {
 		v4l2_dbg(MSG_BUFFER, hdpvr_debug, &dev->v4l2_dev,
-			 "video signal: %dx%d@%dhz\n", vidinf->width,
-			 vidinf->height, vidinf->fps);
-		kfree(vidinf);
+			 "video signal: %dx%d@%dhz\n", vidinf.width,
+			 vidinf.height, vidinf.fps);
 
 		/* start streaming 2 request */
 		ret = usb_control_msg(dev->udev,
@@ -606,21 +605,22 @@ static int vidioc_g_std(struct file *file, void *_fh,
 static int vidioc_querystd(struct file *file, void *_fh, v4l2_std_id *a)
 {
 	struct hdpvr_device *dev = video_drvdata(file);
-	struct hdpvr_video_info *vid_info;
+	struct hdpvr_video_info vid_info;
 	struct hdpvr_fh *fh = _fh;
+	int ret;
 
 	*a = V4L2_STD_ALL;
 	if (dev->options.video_input == HDPVR_COMPONENT)
 		return fh->legacy_mode ? 0 : -ENODATA;
-	vid_info = get_video_info(dev);
-	if (vid_info == NULL)
+	ret = get_video_info(dev, &vid_info);
+	if (ret)
 		return 0;
-	if (vid_info->width == 720 &&
-	    (vid_info->height == 480 || vid_info->height == 576)) {
-		*a = (vid_info->height == 480) ?
+	if (vid_info.width == 720 &&
+	    (vid_info.height == 480 || vid_info.height == 576)) {
+		*a = (vid_info.height == 480) ?
 			V4L2_STD_525_60 : V4L2_STD_625_50;
 	}
-	kfree(vid_info);
+
 	return 0;
 }
 
@@ -665,7 +665,7 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 {
 	struct hdpvr_device *dev = video_drvdata(file);
 	struct hdpvr_fh *fh = _fh;
-	struct hdpvr_video_info *vid_info;
+	struct hdpvr_video_info vid_info;
 	bool interlaced;
 	int ret = 0;
 	int i;
@@ -673,10 +673,10 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 	fh->legacy_mode = false;
 	if (dev->options.video_input)
 		return -ENODATA;
-	vid_info = get_video_info(dev);
-	if (vid_info == NULL)
+	ret = get_video_info(dev, &vid_info);
+	if (ret)
 		return -ENOLCK;
-	interlaced = vid_info->fps <= 30;
+	interlaced = vid_info.fps <= 30;
 	for (i = 0; i < ARRAY_SIZE(hdpvr_dv_timings); i++) {
 		const struct v4l2_bt_timings *bt = &hdpvr_dv_timings[i].bt;
 		unsigned hsize;
@@ -688,17 +688,17 @@ static int vidioc_query_dv_timings(struct file *file, void *_fh,
 			bt->il_vfrontporch + bt->il_vsync + bt->il_vbackporch +
 			bt->height;
 		fps = (unsigned)bt->pixelclock / (hsize * vsize);
-		if (bt->width != vid_info->width ||
-		    bt->height != vid_info->height ||
+		if (bt->width != vid_info.width ||
+		    bt->height != vid_info.height ||
 		    bt->interlaced != interlaced ||
-		    (fps != vid_info->fps && fps + 1 != vid_info->fps))
+		    (fps != vid_info.fps && fps + 1 != vid_info.fps))
 			continue;
 		*timings = hdpvr_dv_timings[i];
 		break;
 	}
 	if (i == ARRAY_SIZE(hdpvr_dv_timings))
 		ret = -ERANGE;
-	kfree(vid_info);
+
 	return ret;
 }
 
@@ -988,6 +988,7 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
 {
 	struct hdpvr_device *dev = video_drvdata(file);
 	struct hdpvr_fh *fh = _fh;
+	int ret;
 
 	/*
 	 * The original driver would always returns the current detected
@@ -1000,14 +1001,13 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *_fh,
 	 * last set format.
 	 */
 	if (fh->legacy_mode) {
-		struct hdpvr_video_info *vid_info;
+		struct hdpvr_video_info vid_info;
 
-		vid_info = get_video_info(dev);
-		if (!vid_info)
+		ret = get_video_info(dev, &vid_info);
+		if (ret)
 			return -EFAULT;
-		f->fmt.pix.width = vid_info->width;
-		f->fmt.pix.height = vid_info->height;
-		kfree(vid_info);
+		f->fmt.pix.width = vid_info.width;
+		f->fmt.pix.height = vid_info.height;
 	} else {
 		f->fmt.pix.width = dev->width;
 		f->fmt.pix.height = dev->height;
diff --git a/drivers/media/usb/hdpvr/hdpvr.h b/drivers/media/usb/hdpvr/hdpvr.h
index 1478f3d..808ea7a 100644
--- a/drivers/media/usb/hdpvr/hdpvr.h
+++ b/drivers/media/usb/hdpvr/hdpvr.h
@@ -303,7 +303,7 @@ int hdpvr_set_audio(struct hdpvr_device *dev, u8 input,
 int hdpvr_config_call(struct hdpvr_device *dev, uint value,
 		      unsigned char valbuf);
 
-struct hdpvr_video_info *get_video_info(struct hdpvr_device *dev);
+int get_video_info(struct hdpvr_device *dev, struct hdpvr_video_info *vid_info);
 
 /* :0 s b8 81 1800 0003 0003 3 < */
 /* :0 0 3 = 0301ff */
-- 
1.7.9.5