Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1687185 > unrolled thread

[PATCH 00/14] gcc-7 warnings

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/14] gcc-7 warnings Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
    [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
      Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Joe Perches <joe@perches.com> - 2017-07-14 12:10 +0200
        Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 12:40 +0200
          Re: [PATCH 05/14] isdn: isdnloop: suppress a gcc-7 warning Kevin Easton <kevin@guarana.org> - 2017-07-15 06:30 +0200
    [PATCH 04/14] x86: math-emu: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
    [PATCH 07/14] proc/kcore: hide a harmless warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:30 +0200
      Re: [PATCH 07/14] proc/kcore: hide a harmless warning Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2017-07-14 14:30 +0200
    [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Joe Perches <joe@perches.com> - 2017-07-14 12:00 +0200
        Re: [PATCH 13/14] iopoll: avoid -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 12:30 +0200
    [PATCH 09/14] SFI: fix tautological-compare warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 11/14] IB/uverbs: fix gcc-7 type warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 11/14] IB/uverbs: fix gcc-7 type warning Leon Romanovsky <leon@kernel.org> - 2017-07-14 11:50 +0200
    [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 10/14] staging:iio:resolver:ad2s1210 fix negative  IIO_ANGL_VEL read Jonathan Cameron <jic23@kernel.org> - 2017-07-15 13:50 +0200
    [PATCH 12/14] drm/nouveau/clk: fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 06/14] acpi: thermal: fix gcc-6/ccache warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
    [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:40 +0200
      Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-14 21:30 +0200
        Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 22:20 +0200
          Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context  warning Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2017-07-14 23:50 +0200
    [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 11:50 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 14:10 +0200
        Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 15:00 +0200
            Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 15:20 +0200
              Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-14 21:40 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-14 14:50 +0200
      Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 15:50 +0200
        Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-17 16:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 16:40 +0200
            Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Arnd Bergmann <arnd@arndb.de> - 2017-07-17 23:30 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Dan Carpenter <dan.carpenter@oracle.com> - 2017-07-17 16:40 +0200
          Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result  interpreted as bool Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 16:40 +0200
    Re: [PATCH 00/14] gcc-7 warnings Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-07-14 12:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1687642 — Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning

On Fri, Jul 14, 2017 at 9:24 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, Jul 14, 2017 at 2:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> FIFO_MODE is an macro expression with a '<<' operator, which
>> gcc points out could be misread as a '<':
>
> Yeah, no, NAK again.
>
> We don't make the code look worse just because gcc is being a f*cking
> moron about things.
>
> This warning is clearly pure garbage.
>

I looked at this one again and found a better approach, matching the
check that is done a few lines later. Unless you find something wrong
with that one, I'd resubmit it with the fixup below.

      Arnd

--- a/drivers/input/misc/adxl34x.c
+++ b/drivers/input/misc/adxl34x.c
@@ -789,21 +789,21 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
                __set_bit(pdata->ev_code_ff, input_dev->keybit);
        }

        if (pdata->ev_code_act_inactivity)
                __set_bit(pdata->ev_code_act_inactivity, input_dev->keybit);

        ac->int_mask |= ACTIVITY | INACTIVITY;

        if (pdata->watermark) {
                ac->int_mask |= WATERMARK;
-               if (FIFO_MODE(pdata->fifo_mode) == 0)
+               if (FIFO_MODE(pdata->fifo_mode) == FIFO_BYPASS)
                        ac->pdata.fifo_mode |= FIFO_STREAM;
        } else {
                ac->int_mask |= DATA_READY;
        }

        if (pdata->tap_axis_control & (TAP_X_EN | TAP_Y_EN | TAP_Z_EN))
                ac->int_mask |= SINGLE_TAP | DOUBLE_TAP;

        if (FIFO_MODE(pdata->fifo_mode) == FIFO_BYPASS)
                ac->fifo_delay = false;

[toc] | [prev] | [next] | [standalone]

#1687680 — Re: [PATCH 08/14] Input: adxl34x - fix gcc-7 -Wint-in-bool-context warning

On Fri, Jul 14, 2017 at 10:17:10PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 14, 2017 at 9:24 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Fri, Jul 14, 2017 at 2:25 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> FIFO_MODE is an macro expression with a '<<' operator, which
> >> gcc points out could be misread as a '<':
> >
> > Yeah, no, NAK again.
> >
> > We don't make the code look worse just because gcc is being a f*cking
> > moron about things.
> >
> > This warning is clearly pure garbage.
> >
> 
> I looked at this one again and found a better approach, matching the
> check that is done a few lines later. Unless you find something wrong
> with that one, I'd resubmit it with the fixup below.
> 
>       Arnd
> 
> --- a/drivers/input/misc/adxl34x.c
> +++ b/drivers/input/misc/adxl34x.c
> @@ -789,21 +789,21 @@ struct adxl34x *adxl34x_probe(struct device *dev, int irq,
>                 __set_bit(pdata->ev_code_ff, input_dev->keybit);
>         }
> 
>         if (pdata->ev_code_act_inactivity)
>                 __set_bit(pdata->ev_code_act_inactivity, input_dev->keybit);
> 
>         ac->int_mask |= ACTIVITY | INACTIVITY;
> 
>         if (pdata->watermark) {
>                 ac->int_mask |= WATERMARK;
> -               if (FIFO_MODE(pdata->fifo_mode) == 0)
> +               if (FIFO_MODE(pdata->fifo_mode) == FIFO_BYPASS)

This is better, not because of GCC, but it makes sense logically; 0 is
not a special value here.

Still, I am not sure that GCC is being that helpful here. Checking
result of shift for 0/non 0 with "!" is very common pattern.

Thanks.

-- 
Dmitry

[toc] | [prev] | [next] | [standalone]

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

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;
 
 	vbifmt->service_set = cx18_get_service_set(vbifmt);
 	return 0;
diff --git a/drivers/media/pci/saa7146/mxb.c b/drivers/media/pci/saa7146/mxb.c
index 504d78807639..d2d843c38579 100644
--- a/drivers/media/pci/saa7146/mxb.c
+++ b/drivers/media/pci/saa7146/mxb.c
@@ -525,8 +525,9 @@ static int vidioc_s_input(struct file *file, void *fh, unsigned int input)
 		return err;
 
 	/* switch video in saa7111a */
-	if (saa7111a_call(mxb, video, s_routing, i, SAA7111_FMT_CCIR, 0))
-		pr_err("VIDIOC_S_INPUT: could not address saa7111a\n");
+	err = saa7111a_call(mxb, video, s_routing, i, SAA7111_FMT_CCIR, 0);
+	if (err)
+		pr_err("VIDIOC_S_INPUT: could not address saa7111a: %d\n", err);
 
 	mxb->cur_audinput = video_audio_connect[input];
 	/* switch the audio-source only if necessary */
diff --git a/drivers/media/platform/atmel/atmel-isc.c b/drivers/media/platform/atmel/atmel-isc.c
index d6534252cdcd..704b34a0cc00 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1475,8 +1475,8 @@ static int isc_formats_init(struct isc_device *isc)
 		fmt++;
 	}
 
-	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
-	       NULL, &mbus_code)) {
+	while (v4l2_subdev_call(subdev, pad, enum_mbus_code,
+	       NULL, &mbus_code) == 0) {
 		mbus_code.index++;
 		fmt = find_format_by_code(mbus_code.code, &i);
 		if (!fmt)
diff --git a/drivers/media/platform/atmel/atmel-isi.c b/drivers/media/platform/atmel/atmel-isi.c
index 891fa2505efa..30b7e6f298ed 100644
--- a/drivers/media/platform/atmel/atmel-isi.c
+++ b/drivers/media/platform/atmel/atmel-isi.c
@@ -1013,8 +1013,8 @@ static int isi_formats_init(struct atmel_isi *isi)
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
 
-	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
-				 NULL, &mbus_code)) {
+	while (v4l2_subdev_call(subdev, pad, enum_mbus_code,
+				NULL, &mbus_code) == 0) {
 		for (i = 0; i < ARRAY_SIZE(isi_formats); i++) {
 			if (isi_formats[i].mbus_code != mbus_code.code)
 				continue;
diff --git a/drivers/media/platform/blackfin/bfin_capture.c b/drivers/media/platform/blackfin/bfin_capture.c
index 1c5166df46f5..864c98f21a0e 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -157,8 +157,8 @@ static int bcap_init_sensor_formats(struct bcap_device *bcap_dev)
 	unsigned int num_formats = 0;
 	int i, j;
 
-	while (!v4l2_subdev_call(bcap_dev->sd, pad,
-				enum_mbus_code, NULL, &code)) {
+	while (v4l2_subdev_call(bcap_dev->sd, pad,
+				enum_mbus_code, NULL, &code) == 0) {
 		num_formats++;
 		code.index++;
 	}
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 7207558d722c..a94157461f58 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -1132,6 +1132,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	unsigned int sph;
 	u32 syn_mode;
 	u32 ccdc_pattern;
+	int ret;
 
 	ccdc->bt656 = false;
 	ccdc->fields = 0;
@@ -1140,7 +1141,6 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	sensor = media_entity_to_v4l2_subdev(pad->entity);
 	if (ccdc->input == CCDC_INPUT_PARALLEL) {
 		struct v4l2_mbus_config cfg;
-		int ret;
 
 		ret = v4l2_subdev_call(sensor, video, g_mbus_config, &cfg);
 		if (!ret)
@@ -1158,7 +1158,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	 */
 	fmt_src.pad = pad->index;
 	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
-	if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
+	ret = v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src);
+	if (!ret) {
 		fmt_info = omap3isp_video_format_info(fmt_src.format.code);
 		depth_in = fmt_info->width;
 	}
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index 399095170b6e..5236c7b171ea 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -763,7 +763,8 @@ static struct soc_camera_format_xlate *pxa_mbus_build_fmts_xlate(
 	};
 	struct soc_camera_format_xlate *user_formats;
 
-	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code)) {
+	while (v4l2_subdev_call(subdev, pad, enum_mbus_code, NULL, &code) ==
+               0) {
 		raw_fmts++;
 		code.index++;
 	}
diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
index 77dff047c41c..a41f4a3d9b69 100644
--- a/drivers/media/platform/rcar-vin/rcar-core.c
+++ b/drivers/media/platform/rcar-vin/rcar-core.c
@@ -54,7 +54,7 @@ static bool rvin_mbus_supported(struct rvin_graph_entity *entity)
 
 	code.index = 0;
 	code.pad = entity->source_pad;
-	while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
+	while (v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code) == 0) {
 		code.index++;
 		switch (code.code) {
 		case MEDIA_BUS_FMT_YUYV8_1X16:
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
index b136844499f6..ee16e9886041 100644
--- a/drivers/media/platform/rcar-vin/rcar-dma.c
+++ b/drivers/media/platform/rcar-vin/rcar-dma.c
@@ -143,6 +143,7 @@ static int rvin_setup(struct rvin_dev *vin)
 	u32 vnmc, dmr, dmr2, interrupts;
 	v4l2_std_id std;
 	bool progressive = false, output_is_yuv = false, input_is_yuv = false;
+	int ret;
 
 	switch (vin->format.field) {
 	case V4L2_FIELD_TOP:
@@ -155,7 +156,8 @@ static int rvin_setup(struct rvin_dev *vin)
 		/* Default to TB */
 		vnmc = VNMC_IM_FULL;
 		/* Use BT if video standard can be read and is 60 Hz format */
-		if (!v4l2_subdev_call(vin_to_source(vin), video, g_std, &std)) {
+		ret = v4l2_subdev_call(vin_to_source(vin), video, g_std, &std);
+		if (ret) {
 			if (std & V4L2_STD_525_60)
 				vnmc = VNMC_IM_FULL | VNMC_FOC;
 		}
diff --git a/drivers/media/platform/soc_camera/soc_camera.c b/drivers/media/platform/soc_camera/soc_camera.c
index 45a0429d75bb..3ef648fc2db6 100644
--- a/drivers/media/platform/soc_camera/soc_camera.c
+++ b/drivers/media/platform/soc_camera/soc_camera.c
@@ -454,7 +454,7 @@ static int soc_camera_init_user_formats(struct soc_camera_device *icd)
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
 
-	while (!v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code)) {
+	while (v4l2_subdev_call(sd, pad, enum_mbus_code, NULL, &code) == 0) {
 		raw_fmts++;
 		code.index++;
 	}
@@ -1202,7 +1202,7 @@ static int soc_camera_probe_finish(struct soc_camera_device *icd)
 		goto evidstart;
 
 	/* Try to improve our guess of a reasonable window format */
-	if (!v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt)) {
+	if (v4l2_subdev_call(sd, pad, get_fmt, NULL, &fmt) == 0) {
 		icd->user_width		= mf->width;
 		icd->user_height	= mf->height;
 		icd->colorspace		= mf->colorspace;
diff --git a/drivers/media/platform/stm32/stm32-dcmi.c b/drivers/media/platform/stm32/stm32-dcmi.c
index 83d32a5d0f40..96084dfd5d11 100644
--- a/drivers/media/platform/stm32/stm32-dcmi.c
+++ b/drivers/media/platform/stm32/stm32-dcmi.c
@@ -1034,8 +1034,8 @@ static int dcmi_formats_init(struct stm32_dcmi *dcmi)
 		.which = V4L2_SUBDEV_FORMAT_ACTIVE,
 	};
 
-	while (!v4l2_subdev_call(subdev, pad, enum_mbus_code,
-				 NULL, &mbus_code)) {
+	while (v4l2_subdev_call(subdev, pad, enum_mbus_code,
+				 NULL, &mbus_code) == 0) {
 		for (i = 0; i < ARRAY_SIZE(dcmi_formats); i++) {
 			if (dcmi_formats[i].mbus_code != mbus_code.code)
 				continue;
diff --git a/drivers/media/platform/ti-vpe/cal.c b/drivers/media/platform/ti-vpe/cal.c
index 177faa36bc16..df0216a6367c 100644
--- a/drivers/media/platform/ti-vpe/cal.c
+++ b/drivers/media/platform/ti-vpe/cal.c
@@ -1348,9 +1348,11 @@ static void cal_stop_streaming(struct vb2_queue *vq)
 	struct cal_dmaqueue *dma_q = &ctx->vidq;
 	struct cal_buffer *buf, *tmp;
 	unsigned long flags;
+	int ret;
 
-	if (v4l2_subdev_call(ctx->sensor, video, s_stream, 0))
-		ctx_err(ctx, "stream off failed in subdev\n");
+	ret = v4l2_subdev_call(ctx->sensor, video, s_stream, 0);
+	if (ret)
+		ctx_err(ctx, "stream off failed in subdev: %d\n", ret);
 
 	csi2_ppi_disable(ctx);
 	disable_irqs(ctx);
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);
 
-	return 0;
+	return ret;
 }
 
 int atomisp_flash_enable(struct atomisp_sub_device *asd, int num_frames)
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687293 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

Changing:

- if (!frob()) {
+ if (frob() == 0) {

is a totally pointless change.  They're both bad, because they're doing
success testing instead of failure testing, but probably the second one
is slightly worse.

This warning seems dumb.  I can't imagine it has even a 10% success rate
at finding real bugs.  Just disable it.

Changing the code to propagate error codes, is the right thing of course
so long as it doesn't introduce bugs.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1687331 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Fri, Jul 14, 2017 at 2:05 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Changing:
>
> - if (!frob()) {
> + if (frob() == 0) {
>
> is a totally pointless change.  They're both bad, because they're doing
> success testing instead of failure testing, but probably the second one
> is slightly worse.
>
> This warning seems dumb.  I can't imagine it has even a 10% success rate
> at finding real bugs.  Just disable it.
>
> Changing the code to propagate error codes, is the right thing of course
> so long as it doesn't introduce bugs.

It found a two of bugs that I fixed earlier:

f0e8faa7a5e8 ("ARM: ux500: fix prcmu_is_cpu_in_wfi() calculation")
af15769ffab1 ("scsi: mvsas: fix command_active typo")

plus three patches from this series:

1. staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read
2. isdn: isdnloop: suppress a gcc-7 warning (my patch is wrong,
   as Joe pointed out there is a real bug)
3. drm/vmwgfx: avoid gcc-7 parentheses (here, Linus had a better
   analysis of the problem, so we should consider that a bug as well)

I would estimate around 25% success rate here, which isn't that
bad for a new warning.

I agree that most of the false positives are really dumb though.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1687367 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

Ah...  I see why it's complaining about these ones in particular...

I don't agree with it as a static analysis dev, and I don't like the
changes too much.  But since it's only generating a hand full of
warnings then I don't care.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1687411 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Fri, Jul 14, 2017 at 03:55:26PM +0300, Dan Carpenter wrote:
> I don't agree with it as a static analysis dev...

What I mean is if it's a macro that returns -ENODEV or a function that
returns -ENODEV, they should both be treated the same.  The other
warnings this check prints are quite clever.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1687624 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Fri, Jul 14, 2017 at 3:09 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> On Fri, Jul 14, 2017 at 03:55:26PM +0300, Dan Carpenter wrote:
>> I don't agree with it as a static analysis dev...
>
> What I mean is if it's a macro that returns -ENODEV or a function that
> returns -ENODEV, they should both be treated the same.  The other
> warnings this check prints are quite clever.

I think this is what gcc tries to do, and it should work normally, but it
fails when using ccache. I know I had cases like that, not entirely sure
if this is one of them. Maybe it just means I should give up on using
ccache in preprocessor mode.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1687365 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Fri, Jul 14, 2017 at 11:36:56AM +0200, Arnd Bergmann wrote:
> @@ -1158,7 +1158,8 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
>  	 */
>  	fmt_src.pad = pad->index;
>  	fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> -	if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
> +	ret = v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src);
> +	if (!ret) {
>  		fmt_info = omap3isp_video_format_info(fmt_src.format.code);
>  		depth_in = fmt_info->width;
>  	}

Is the original code buggy?

media/platform/omap3isp/ispccdc.c
  1156          /* Compute the lane shifter shift value and enable the bridge when the
  1157           * input format is a non-BT.656 YUV variant.
  1158           */
  1159          fmt_src.pad = pad->index;
  1160          fmt_src.which = V4L2_SUBDEV_FORMAT_ACTIVE;
  1161          if (!v4l2_subdev_call(sensor, pad, get_fmt, NULL, &fmt_src)) {
  1162                  fmt_info = omap3isp_video_format_info(fmt_src.format.code);
  1163                  depth_in = fmt_info->width;
  1164          }

If v4l2_subdev_call() then depth_in is zero.

  1165  
  1166          fmt_info = omap3isp_video_format_info(format->code);
  1167          depth_out = fmt_info->width;
  1168          shift = depth_in - depth_out;
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

How do we know that this subtraction doesn't set "shift" to a very high
positive?

  1169  
  1170          if (ccdc->bt656)
  1171                  bridge = ISPCTRL_PAR_BRIDGE_DISABLE;
  1172          else if (fmt_info->code == MEDIA_BUS_FMT_YUYV8_2X8)
  1173                  bridge = ISPCTRL_PAR_BRIDGE_LENDIAN;
  1174          else if (fmt_info->code == MEDIA_BUS_FMT_UYVY8_2X8)
  1175                  bridge = ISPCTRL_PAR_BRIDGE_BENDIAN;
  1176          else
  1177                  bridge = ISPCTRL_PAR_BRIDGE_DISABLE;
  1178  
  1179          omap3isp_configure_bridge(isp, ccdc->input, parcfg, shift, bridge);

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1689065 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On 14/07/17 11:36, Arnd Bergmann wrote:
> 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>

> 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

[toc] | [prev] | [next] | [standalone]

#1689099 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Mon, Jul 17, 2017 at 3:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 14/07/17 11:36, Arnd Bergmann wrote:
>> @@ -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.

I think Dan was recommending the opposite here, if I understood you
both correctly:
he said we should propagate the error code unless we know it's wrong, while you
want to keep the current behavior to avoid introducing changes ;-)

I guess in either case, looking at the callers more carefully would be
a good idea.

>> -     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.

it's possible that this only happened with 'ccache', which first preprocesses
the source and the passes it with v4l2_subdev_call expanded into the
compiler. This means the line looks like

        if ((!(cx->sd_av) ? -ENODEV :
            (((cx->sd_av)->ops->vbi && (cx->sd_av)->ops->vbi->g_sliced_fmt) ?
               (cx->sd_av)->ops->vbi->g_sliced_fmt(cx->sd_av)),
&fmt->fmt.sliced) :
               -ENOIOCTLCMD))

The compiler now complains about the sub-expression that it sees for
cx->sd_av==NULL:

   if (-ENODEV)

which it considers nonsense because it is always true and the value gets
ignored.

Let me try again without ccache for now and see what warnings remain.
We can find a solution for those first, and then decide how to deal with
ccache.

        Arnd

[toc] | [prev] | [next] | [standalone]

#1689114 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On 17/07/17 16:26, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 3:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 14/07/17 11:36, Arnd Bergmann wrote:
>>> @@ -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.
> 
> I think Dan was recommending the opposite here, if I understood you
> both correctly:
> he said we should propagate the error code unless we know it's wrong, while you
> want to keep the current behavior to avoid introducing changes ;-)
> 
> I guess in either case, looking at the callers more carefully would be
> a good idea.

The subtle problem here is that v4l2_subdev_call will return -ENOIOCTLCMD if
ops->vbi->g_sliced_fmt == NULL, which typically is not returned to userspace
but either ignored or replaced by another error. It indicates that the
sub device doesn't implement this operation, and it depends on the context
and the operation whether or not that is to be considered an error.

I have no clue what is expected here, without digging deep in the code.

Better to keep it as-is. It really isn't important to waste time on this.

> 
>>> -     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.
> 
> it's possible that this only happened with 'ccache', which first preprocesses
> the source and the passes it with v4l2_subdev_call expanded into the
> compiler. This means the line looks like
> 
>         if ((!(cx->sd_av) ? -ENODEV :
>             (((cx->sd_av)->ops->vbi && (cx->sd_av)->ops->vbi->g_sliced_fmt) ?
>                (cx->sd_av)->ops->vbi->g_sliced_fmt(cx->sd_av)),
> &fmt->fmt.sliced) :
>                -ENOIOCTLCMD))
> 
> The compiler now complains about the sub-expression that it sees for
> cx->sd_av==NULL:
> 
>    if (-ENODEV)
> 
> which it considers nonsense because it is always true and the value gets
> ignored.
> 
> Let me try again without ccache for now and see what warnings remain.
> We can find a solution for those first, and then decide how to deal with
> ccache.

Sounds good.

I'm OK with applying this if there is no other way to prevent these warnings.

Regards,

	Hans

> 
>         Arnd
> 

[toc] | [prev] | [next] | [standalone]

#1689493 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Mon, Jul 17, 2017 at 4:35 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 17/07/17 16:26, Arnd Bergmann wrote:

>> Let me try again without ccache for now and see what warnings remain.
>> We can find a solution for those first, and then decide how to deal with
>> ccache.
>
> Sounds good.
>
> I'm OK with applying this if there is no other way to prevent these warnings.

Small update: I noticed that having ccache being the default compiler
even with CCACHE_DISABLE=1 causes a lot of these warnings. Completely
taking ccache out of the picture however seems to have eliminated the
warnings about v4l2_subdev_call() and other silly warnings, but not
the interesting ones in the -Wint-in-bool-context category.

       Arnd

[toc] | [prev] | [next] | [standalone]

#1689117 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On Mon, Jul 17, 2017 at 04:26:23PM +0200, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 3:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> > On 14/07/17 11:36, Arnd Bergmann wrote:
> >> @@ -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.
> 
> I think Dan was recommending the opposite here, if I understood you
> both correctly:
> he said we should propagate the error code unless we know it's wrong, while you
> want to keep the current behavior to avoid introducing changes ;-)
> 

I don't know the subsystem rules at all, so don't listen to me.

regards,
dan carpenter

[toc] | [prev] | [next] | [standalone]

#1689130 — Re: [PATCH 14/14] [media] fix warning on v4l2_subdev_call() result interpreted as bool

On 17/07/17 16:26, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 3:45 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 14/07/17 11:36, Arnd Bergmann wrote:
>>> @@ -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.
> 
> I think Dan was recommending the opposite here, if I understood you
> both correctly:
> he said we should propagate the error code unless we know it's wrong, while you
> want to keep the current behavior to avoid introducing changes ;-)
> 
> I guess in either case, looking at the callers more carefully would be
> a good idea.

The subtle problem here is that v4l2_subdev_call will return -ENOIOCTLCMD if
ops->vbi->g_sliced_fmt == NULL, which typically is not returned to userspace
but either ignored or replaced by another error. It indicates that the
sub device doesn't implement this operation, and it depends on the context
and the operation whether or not that is to be considered an error.

I have no clue what is expected here, without digging deep in the code.

Better to keep it as-is. It really isn't important to waste time on this.

> 
>>> -     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.
> 
> it's possible that this only happened with 'ccache', which first preprocesses
> the source and the passes it with v4l2_subdev_call expanded into the
> compiler. This means the line looks like
> 
>         if ((!(cx->sd_av) ? -ENODEV :
>             (((cx->sd_av)->ops->vbi && (cx->sd_av)->ops->vbi->g_sliced_fmt) ?
>                (cx->sd_av)->ops->vbi->g_sliced_fmt(cx->sd_av)),
> &fmt->fmt.sliced) :
>                -ENOIOCTLCMD))
> 
> The compiler now complains about the sub-expression that it sees for
> cx->sd_av==NULL:
> 
>    if (-ENODEV)
> 
> which it considers nonsense because it is always true and the value gets
> ignored.
> 
> Let me try again without ccache for now and see what warnings remain.
> We can find a solution for those first, and then decide how to deal with
> ccache.

Sounds good.

I'm OK with applying this if there is no other way to prevent these warnings.

Regards,

	Hans

> 
>         Arnd
> 

[toc] | [prev] | [next] | [standalone]

#1687239

On Fri, Jul 14, 2017 at 11:25:12AM +0200, Arnd Bergmann wrote:
> This series should shut up all warnings introduced by gcc-6 or gcc-7 on
> today's linux-next, as observed in "allmodconfig" builds on x86,
> arm and arm64.
> 
> I have sent some of these before, but some others are new, as I had
> at some point disabled the -Wint-in-bool-context warning in my
> randconfig testing and did not notice the other warnings.
> 
> I have another series to address all -Wformat-overflow warnings,
> and one more patch to turn off the -Wformat-truncation warnings
> unless we build with "make W=1". I'll send that separately.
> 
> Most of these are consist of trivial refactoring of the code to
> shut up false-positive warnings, the one exception being
> "staging:iio:resolver:ad2s1210 fix negative IIO_ANGL_VEL read",
> which fixes a regression against linux-3.1 that has gone
> unnoticed since then. Still, review from subsystem maintainers
> would be appreciated.
> 
> I would suggest that Andrew Morton can pick these up into linux-mm
> so we can make sure they all make it into the release. Alternatively
> Linus might feel like picking them all up himself.
> 
> While I did not mark the harmless ones for stable backports,
> Greg may also want to pick them up once they go upstream, to
> help build-test the stable kernels with gcc-7.

Thanks for these, I'll keep an eye out for them to get into the stable
trees, so I can eventually update my test-build box to gcc-7.

thanks,

greg k-h

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web