Thread (13 messages) 13 messages, 3 authors, 2017-07-17

[PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

From: Hans Verkuil <hidden>
Date: 2017-07-17 13:45:18
Also in: dri-devel, linux-ide, linux-media, linux-renesas-soc, lkml

On 14/07/17 11:36, Arnd Bergmann wrote:
quoted hunk ↗ jump to hunk
v4l2_subdev_call is a macro returning whatever the callback return
type is, usually 'int'. With gcc-7 and ccache, this can lead to
many wanings like:

media/platform/pxa_camera.c: In function 'pxa_mbus_build_fmts_xlate':
media/platform/pxa_camera.c:766:27: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]
  while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
media/atomisp/pci/atomisp2/atomisp_cmd.c: In function 'atomisp_s_ae_window':
media/atomisp/pci/atomisp2/atomisp_cmd.c:6414:52: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]
  if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,

The best workaround I could come up with is to change all the
callers that use the return code from v4l2_subdev_call() in an
'if' or 'while' condition.

In case of simple 'if' checks, adding a temporary variable is
usually ok, and sometimes this can be used to propagate or
print an error code, so I do that.

For the 'while' loops, I ended up adding an otherwise useless
comparison with zero, which unfortunately makes the code a little
uglied.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/pci/cx18/cx18-ioctl.c                      |  6 ++++--
 drivers/media/pci/saa7146/mxb.c                          |  5 +++--
 drivers/media/platform/atmel/atmel-isc.c                 |  4 ++--
 drivers/media/platform/atmel/atmel-isi.c                 |  4 ++--
 drivers/media/platform/blackfin/bfin_capture.c           |  4 ++--
 drivers/media/platform/omap3isp/ispccdc.c                |  5 +++--
 drivers/media/platform/pxa_camera.c                      |  3 ++-
 drivers/media/platform/rcar-vin/rcar-core.c              |  2 +-
 drivers/media/platform/rcar-vin/rcar-dma.c               |  4 +++-
 drivers/media/platform/soc_camera/soc_camera.c           |  4 ++--
 drivers/media/platform/stm32/stm32-dcmi.c                |  4 ++--
 drivers/media/platform/ti-vpe/cal.c                      |  6 ++++--
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c | 13 +++++++------
 13 files changed, 37 insertions(+), 27 deletions(-)
diff --git a/drivers/media/pci/cx18/cx18-ioctl.c b/drivers/media/pci/cx18/cx18-ioctl.c
index 80b902b12a78..1803f28fc501 100644
--- a/drivers/media/pci/cx18/cx18-ioctl.c
+++ b/drivers/media/pci/cx18/cx18-ioctl.c
@@ -188,6 +188,7 @@ static int cx18_g_fmt_sliced_vbi_cap(struct file *file, void *fh,
 {
 	struct cx18 *cx = fh2id(fh)->cx;
 	struct v4l2_sliced_vbi_format *vbifmt = &fmt->fmt.sliced;
+	int ret;
 
 	/* sane, V4L2 spec compliant, defaults */
 	vbifmt->reserved[0] = 0;
@@ -201,8 +202,9 @@ static int cx18_g_fmt_sliced_vbi_cap(struct file *file, void *fh,
 	 * digitizer/slicer.  Note, cx18_av_vbi() wipes the passed in
 	 * fmt->fmt.sliced under valid calling conditions
 	 */
-	if (v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, &fmt->fmt.sliced))
-		return -EINVAL;
+	ret = v4l2_subdev_call(cx->sd_av, vbi, g_sliced_fmt, &fmt->fmt.sliced);
+	if (ret)
+		return ret;
Please keep the -EINVAL here. I can't be 100% certain that returning 'ret' wouldn't
break something.
 
 	vbifmt->service_set = cx18_get_service_set(vbifmt);
 	return 0;
<snip>
quoted hunk ↗ jump to hunk
diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
index 97093baf28ac..fe56a037f065 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c
@@ -6405,19 +6405,20 @@ int atomisp_s_ae_window(struct atomisp_sub_device *asd,
 	struct atomisp_device *isp = asd->isp;
 	/* Coverity CID 298071 - initialzize struct */
 	struct v4l2_subdev_selection sel = { 0 };
+	int ret;
 
 	sel.r.left = arg->x_left;
 	sel.r.top = arg->y_top;
 	sel.r.width = arg->x_right - arg->x_left + 1;
 	sel.r.height = arg->y_bottom - arg->y_top + 1;
 
-	if (v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
-			     pad, set_selection, NULL, &sel)) {
-		dev_err(isp->dev, "failed to call sensor set_selection.\n");
-		return -EINVAL;
-	}
+	ret = v4l2_subdev_call(isp->inputs[asd->input_curr].camera,
+			       pad, set_selection, NULL, &sel);
+	if (ret)
+		dev_err(isp->dev, "failed to call sensor set_selection: %d\n",
+			ret);
Same here: just keep the 'return -EINVAL'.
 
-	return 0;
+	return ret;
 }
 
 int atomisp_flash_enable(struct atomisp_sub_device *asd, int num_frames)
This is all very hackish, though. I'm not terribly keen on this patch. It's not
clear to me *why* these warnings appear in your setup.

Regards,

	Hans
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help