[PATCH 0/7] Various smatch fixes

[PATCH 0/7] Various smatch fixes

[Hans Verkuil]

The daily build produces a number of smatch warnings.

This series fixes those warnings.

Regards,

	Hans

Hans Verkuil (7):
  v4l2-subdev.c: kvmalloc_array -> kvcalloc
  v4l2-ctls-core.c: kvmalloc_array -> kvcalloc
  videobuf2-dma-sg.c: kvmalloc_array -> kvcalloc
  cx25821: request_irq -> devm_request_irq
  ccs-core.c: fix failure to call clk_disable_unprepare
  s5k6a3: add missing clk_disable_unprepare
  uvc_v4l2.c: avoid using iterator used outside loop

 drivers/media/common/videobuf2/videobuf2-dma-sg.c |  3 +--
 drivers/media/i2c/ccs/ccs-core.c                  |  7 +++++--
 drivers/media/i2c/s5k6a3.c                        |  4 +++-
 drivers/media/pci/cx25821/cx25821-alsa.c          |  4 ++--
 drivers/media/usb/uvc/uvc_v4l2.c                  | 13 +++++++++----
 drivers/media/v4l2-core/v4l2-ctrls-core.c         |  5 ++---
 drivers/media/v4l2-core/v4l2-subdev.c             |  5 ++---
 7 files changed, 24 insertions(+), 17 deletions(-)

-- 
2.34.1

[PATCH 1/7] v4l2-subdev.c: kvmalloc_array -> kvcalloc

[Hans Verkuil]

Fixes smatch warning:

drivers/media/v4l2-core/v4l2-subdev.c:978 __v4l2_subdev_state_alloc() warn: Please consider using kvcalloc instead

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 3da200bb23dd..5c27bac772ea 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -975,9 +975,8 @@ __v4l2_subdev_state_alloc(struct v4l2_subdev *sd, const char *lock_name,
 		state->lock = &state->_lock;
 
 	if (sd->entity.num_pads) {
-		state->pads = kvmalloc_array(sd->entity.num_pads,
-					     sizeof(*state->pads),
-					     GFP_KERNEL | __GFP_ZERO);
+		state->pads = kvcalloc(sd->entity.num_pads,
+				       sizeof(*state->pads), GFP_KERNEL);
 		if (!state->pads) {
 			ret = -ENOMEM;
 			goto err;
-- 
2.34.1

Re: [PATCH 1/7] v4l2-subdev.c: kvmalloc_array -> kvcalloc

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Mon, May 09, 2022 at 11:15:47AM +0200, Hans Verkuil wrote:
> Fixes smatch warning:
> 
> drivers/media/v4l2-core/v4l2-subdev.c:978 __v4l2_subdev_state_alloc() warn: Please consider using kvcalloc instead
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 3da200bb23dd..5c27bac772ea 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -975,9 +975,8 @@ __v4l2_subdev_state_alloc(struct v4l2_subdev *sd, const char *lock_name,
>  		state->lock = &state->_lock;
>  
>  	if (sd->entity.num_pads) {
> -		state->pads = kvmalloc_array(sd->entity.num_pads,
> -					     sizeof(*state->pads),
> -					     GFP_KERNEL | __GFP_ZERO);
> +		state->pads = kvcalloc(sd->entity.num_pads,
> +				       sizeof(*state->pads), GFP_KERNEL);
>  		if (!state->pads) {
>  			ret = -ENOMEM;
>  			goto err;

-- 
Regards,

Laurent Pinchart

[PATCH 2/7] v4l2-ctls-core.c: kvmalloc_array -> kvcalloc

[Hans Verkuil]

Fixes smatch warning:

drivers/media/v4l2-core/v4l2-ctrls-core.c:1143 v4l2_ctrl_handler_init_class() warn: Please consider using kvcalloc instead

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/v4l2-core/v4l2-ctrls-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 8968cec8454e..949c1884d9c1 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -1140,9 +1140,8 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl,
 	INIT_LIST_HEAD(&hdl->ctrls);
 	INIT_LIST_HEAD(&hdl->ctrl_refs);
 	hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8;
-	hdl->buckets = kvmalloc_array(hdl->nr_of_buckets,
-				      sizeof(hdl->buckets[0]),
-				      GFP_KERNEL | __GFP_ZERO);
+	hdl->buckets = kvcalloc(hdl->nr_of_buckets, sizeof(hdl->buckets[0]),
+				GFP_KERNEL);
 	hdl->error = hdl->buckets ? 0 : -ENOMEM;
 	v4l2_ctrl_handler_init_request(hdl);
 	return hdl->error;
-- 
2.34.1

Re: [PATCH 2/7] v4l2-ctls-core.c: kvmalloc_array -> kvcalloc

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Mon, May 09, 2022 at 11:15:48AM +0200, Hans Verkuil wrote:
> Fixes smatch warning:
> 
> drivers/media/v4l2-core/v4l2-ctrls-core.c:1143 v4l2_ctrl_handler_init_class() warn: Please consider using kvcalloc instead
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/v4l2-core/v4l2-ctrls-core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index 8968cec8454e..949c1884d9c1 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -1140,9 +1140,8 @@ int v4l2_ctrl_handler_init_class(struct v4l2_ctrl_handler *hdl,
>  	INIT_LIST_HEAD(&hdl->ctrls);
>  	INIT_LIST_HEAD(&hdl->ctrl_refs);
>  	hdl->nr_of_buckets = 1 + nr_of_controls_hint / 8;
> -	hdl->buckets = kvmalloc_array(hdl->nr_of_buckets,
> -				      sizeof(hdl->buckets[0]),
> -				      GFP_KERNEL | __GFP_ZERO);
> +	hdl->buckets = kvcalloc(hdl->nr_of_buckets, sizeof(hdl->buckets[0]),
> +				GFP_KERNEL);
>  	hdl->error = hdl->buckets ? 0 : -ENOMEM;
>  	v4l2_ctrl_handler_init_request(hdl);
>  	return hdl->error;

-- 
Regards,

Laurent Pinchart

[PATCH 3/7] videobuf2-dma-sg.c: kvmalloc_array -> kvcalloc

[Hans Verkuil]

Fixes smatch warning:

drivers/media/common/videobuf2/videobuf2-dma-sg.c:129 vb2_dma_sg_alloc() warn: Please consider using kvcalloc instead

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/common/videobuf2/videobuf2-dma-sg.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index f8a21c560ad2..fa69158a65b1 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -126,8 +126,7 @@ static void *vb2_dma_sg_alloc(struct vb2_buffer *vb, struct device *dev,
 	 * there is no memory consistency guarantee, hence dma-sg ignores DMA
 	 * attributes passed from the upper layer.
 	 */
-	buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *),
-				    GFP_KERNEL | __GFP_ZERO);
+	buf->pages = kvcalloc(buf->num_pages, sizeof(struct page *), GFP_KERNEL);
 	if (!buf->pages)
 		goto fail_pages_array_alloc;
 
-- 
2.34.1

Re: [PATCH 3/7] videobuf2-dma-sg.c: kvmalloc_array -> kvcalloc

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Mon, May 09, 2022 at 11:15:49AM +0200, Hans Verkuil wrote:
> Fixes smatch warning:
> 
> drivers/media/common/videobuf2/videobuf2-dma-sg.c:129 vb2_dma_sg_alloc() warn: Please consider using kvcalloc instead
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/common/videobuf2/videobuf2-dma-sg.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> index f8a21c560ad2..fa69158a65b1 100644
> --- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> +++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
> @@ -126,8 +126,7 @@ static void *vb2_dma_sg_alloc(struct vb2_buffer *vb, struct device *dev,
>  	 * there is no memory consistency guarantee, hence dma-sg ignores DMA
>  	 * attributes passed from the upper layer.
>  	 */
> -	buf->pages = kvmalloc_array(buf->num_pages, sizeof(struct page *),
> -				    GFP_KERNEL | __GFP_ZERO);
> +	buf->pages = kvcalloc(buf->num_pages, sizeof(struct page *), GFP_KERNEL);
>  	if (!buf->pages)
>  		goto fail_pages_array_alloc;
>  

-- 
Regards,

Laurent Pinchart

[PATCH 4/7] cx25821: request_irq -> devm_request_irq

[Hans Verkuil]

Fixes smatch warning:

drivers/media/pci/cx25821/cx25821-alsa.c:768 cx25821_audio_initdev() warn: 'dev->pci->irq' from request_irq() not released on lines: 768.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/pci/cx25821/cx25821-alsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/cx25821/cx25821-alsa.c b/drivers/media/pci/cx25821/cx25821-alsa.c
index 438fdcec6eac..a42f0c03a7ca 100644
--- a/drivers/media/pci/cx25821/cx25821-alsa.c
+++ b/drivers/media/pci/cx25821/cx25821-alsa.c
@@ -728,8 +728,8 @@ static int cx25821_audio_initdev(struct cx25821_dev *dev)
 
 	chip->irq = dev->pci->irq;
 
-	err = request_irq(dev->pci->irq, cx25821_irq,
-			  IRQF_SHARED, chip->dev->name, chip);
+	err = devm_request_irq(&dev->pci->dev, dev->pci->irq, cx25821_irq,
+			       IRQF_SHARED, chip->dev->name, chip);
 
 	if (err < 0) {
 		pr_err("ERROR %s: can't get IRQ %d for ALSA\n", chip->dev->name,
-- 
2.34.1

[PATCH 5/7] ccs-core.c: fix failure to call clk_disable_unprepare

[Hans Verkuil]

Fixes smatch warning:

drivers/media/i2c/ccs/ccs-core.c:1676 ccs_power_on() warn: 'sensor->ext_clk' from clk_prepare_enable() not released on lines: 1606.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/ccs/ccs-core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 55bd303f0e18..7609add2aff4 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -1602,8 +1602,11 @@ static int ccs_power_on(struct device *dev)
 			usleep_range(1000, 2000);
 		} while (--retry);
 
-		if (!reset)
-			return -EIO;
+		if (!reset) {
+			dev_err(dev, "software reset failed\n");
+			rval = -EIO;
+			goto out_cci_addr_fail;
+		}
 	}
 
 	if (sensor->hwcfg.i2c_addr_alt) {
-- 
2.34.1

[PATCH 6/7] s5k6a3: add missing clk_disable_unprepare

[Hans Verkuil]

Fix smatch warning:

drivers/media/i2c/s5k6a3.c:234 __s5k6a3_power_on() warn: 'sensor->clock' from clk_prepare_enable() not released on lines: 234.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/i2c/s5k6a3.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/s5k6a3.c b/drivers/media/i2c/s5k6a3.c
index b97dd6149e90..f6ecf6f92bb2 100644
--- a/drivers/media/i2c/s5k6a3.c
+++ b/drivers/media/i2c/s5k6a3.c
@@ -213,7 +213,7 @@ static int __s5k6a3_power_on(struct s5k6a3 *sensor)
 	for (i++; i < S5K6A3_NUM_SUPPLIES; i++) {
 		ret = regulator_enable(sensor->supplies[i].consumer);
 		if (ret < 0)
-			goto error_reg_dis;
+			goto error_clk;
 	}
 
 	gpio_set_value(sensor->gpio_reset, 1);
@@ -226,6 +226,8 @@ static int __s5k6a3_power_on(struct s5k6a3 *sensor)
 	msleep(20);
 	return 0;
 
+error_clk:
+	clk_disable_unprepare(sensor->clock);
 error_reg_dis:
 	for (--i; i >= 0; --i)
 		regulator_disable(sensor->supplies[i].consumer);
-- 
2.34.1

[PATCH 7/7] uvc_v4l2.c: avoid using iterator used outside loop

[Hans Verkuil]

Fixes these two smatch warnings:

drivers/media/usb/uvc/uvc_v4l2.c:885 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'
drivers/media/usb/uvc/uvc_v4l2.c:896 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/media/usb/uvc/uvc_v4l2.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 711556d13d03..ff3f04af4e21 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -871,6 +871,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
 	struct uvc_video_chain *chain = handle->chain;
 	const struct uvc_entity *selector = chain->selector;
 	struct uvc_entity *iterm = NULL;
+	bool found_pin = false;
 	u32 index = input->index;
 	int pin = 0;
 
@@ -879,21 +880,25 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
 		if (index != 0)
 			return -EINVAL;
 		list_for_each_entry(iterm, &chain->entities, chain) {
-			if (UVC_ENTITY_IS_ITERM(iterm))
+			if (UVC_ENTITY_IS_ITERM(iterm)) {
+				pin = iterm->id;
+				found_pin = true;
 				break;
+			}
 		}
-		pin = iterm->id;
 	} else if (index < selector->bNrInPins) {
 		pin = selector->baSourceID[index];
 		list_for_each_entry(iterm, &chain->entities, chain) {
 			if (!UVC_ENTITY_IS_ITERM(iterm))
 				continue;
-			if (iterm->id == pin)
+			if (iterm->id == pin) {
+				found_pin = true;
 				break;
+			}
 		}
 	}
 
-	if (iterm == NULL || iterm->id != pin)
+	if (!found_pin)
 		return -EINVAL;
 
 	memset(input, 0, sizeof(*input));
-- 
2.34.1

Re: [PATCH 7/7] uvc_v4l2.c: avoid using iterator used outside loop

[Sakari Ailus]

Hi Hans,

Thanks for the set.

On Mon, May 09, 2022 at 11:15:53AM +0200, Hans Verkuil wrote:
> Fixes these two smatch warnings:
> 
> drivers/media/usb/uvc/uvc_v4l2.c:885 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'
> drivers/media/usb/uvc/uvc_v4l2.c:896 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 711556d13d03..ff3f04af4e21 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c

...

> @@ -879,21 +880,25 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
>  		if (index != 0)
>  			return -EINVAL;
>  		list_for_each_entry(iterm, &chain->entities, chain) {
> -			if (UVC_ENTITY_IS_ITERM(iterm))
> +			if (UVC_ENTITY_IS_ITERM(iterm)) {
> +				pin = iterm->id;
> +				found_pin = true;
>  				break;
> +			}
>  		}
> -		pin = iterm->id;
>  	} else if (index < selector->bNrInPins) {
>  		pin = selector->baSourceID[index];
>  		list_for_each_entry(iterm, &chain->entities, chain) {
>  			if (!UVC_ENTITY_IS_ITERM(iterm))
>  				continue;
> -			if (iterm->id == pin)
> +			if (iterm->id == pin) {
> +				found_pin = true;
>  				break;
> +			}
>  		}
>  	}
>  
> -	if (iterm == NULL || iterm->id != pin)
> +	if (!found_pin)
>  		return -EINVAL;
>  
>  	memset(input, 0, sizeof(*input));

iterm is used a few lines below, too, so this patch doesn't remove its use
outside the loop. Which btw. I don't think is a problem at all. Doing that
is relatively common IMO.

For the rest of the set:

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

-- 
Kind regards,

Sakari Ailus

Re: [PATCH 7/7] uvc_v4l2.c: avoid using iterator used outside loop

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Mon, May 09, 2022 at 11:15:53AM +0200, Hans Verkuil wrote:
> Fixes these two smatch warnings:
> 
> drivers/media/usb/uvc/uvc_v4l2.c:885 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'
> drivers/media/usb/uvc/uvc_v4l2.c:896 uvc_ioctl_enum_input() warn: iterator used outside loop: 'iterm'
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

This conflicts with 261f33388c29 ("media: uvcvideo: Fix missing check to
determine if element is found in list").

> ---
>  drivers/media/usb/uvc/uvc_v4l2.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index 711556d13d03..ff3f04af4e21 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -871,6 +871,7 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
>  	struct uvc_video_chain *chain = handle->chain;
>  	const struct uvc_entity *selector = chain->selector;
>  	struct uvc_entity *iterm = NULL;
> +	bool found_pin = false;
>  	u32 index = input->index;
>  	int pin = 0;
>  
> @@ -879,21 +880,25 @@ static int uvc_ioctl_enum_input(struct file *file, void *fh,
>  		if (index != 0)
>  			return -EINVAL;
>  		list_for_each_entry(iterm, &chain->entities, chain) {
> -			if (UVC_ENTITY_IS_ITERM(iterm))
> +			if (UVC_ENTITY_IS_ITERM(iterm)) {
> +				pin = iterm->id;
> +				found_pin = true;
>  				break;
> +			}
>  		}
> -		pin = iterm->id;
>  	} else if (index < selector->bNrInPins) {
>  		pin = selector->baSourceID[index];
>  		list_for_each_entry(iterm, &chain->entities, chain) {
>  			if (!UVC_ENTITY_IS_ITERM(iterm))
>  				continue;
> -			if (iterm->id == pin)
> +			if (iterm->id == pin) {
> +				found_pin = true;
>  				break;
> +			}
>  		}
>  	}
>  
> -	if (iterm == NULL || iterm->id != pin)
> +	if (!found_pin)
>  		return -EINVAL;
>  
>  	memset(input, 0, sizeof(*input));

-- 
Regards,

Laurent Pinchart