diff mbox

staging: atomisp: avoid false-positive maybe-uninitialized warning

Message ID 20170328100321.3836826-1-arnd@arndb.de (mailing list archive)
State New, archived
Headers show

Commit Message

Arnd Bergmann March 28, 2017, 10:02 a.m. UTC
In combination with CONFIG_PROFILE_ANNOTATED_BRANCHES=y, the unlikely()
inside of the WARN() macro becomes too complex for gcc to see that
we don't use the output arguments of mt9m114_to_res() are used
correctly:

drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_get_fmt':
drivers/staging/media/atomisp/i2c/mt9m114.c:817:13: error: 'height' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  int width, height;
             ^~~~~~
drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_s_exposure_selection':
drivers/staging/media/atomisp/i2c/mt9m114.c:1179:13: error: 'height' may be used uninitialized in this function [-Werror=maybe-uninitialized]

Without WARN_ON(), there is no problem, so by simply replacing it with
v4l2_err(), the warnings go away. The WARN() output is also not needed
here, as we'd probably catch the problem before even getting here,
and other checks for the same condition already use v4l2_err.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/staging/media/atomisp/i2c/mt9m114.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

kernel test robot March 28, 2017, 8:10 p.m. UTC | #1
Hi Arnd,

[auto build test ERROR on staging/staging-testing]
[also build test ERROR on next-20170328]
[cannot apply to linuxtv-media/master v4.11-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-atomisp-avoid-false-positive-maybe-uninitialized-warning/20170329-023715
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_get_fmt':
>> drivers/staging/media/atomisp/i2c/mt9m114.c:818:25: error: unused variable 'dev' [-Werror=unused-variable]
     struct mt9m114_device *dev = to_mt9m114_sensor(sd);
                            ^~~
   drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_s_exposure_selection':
   drivers/staging/media/atomisp/i2c/mt9m114.c:1179:25: error: unused variable 'dev' [-Werror=unused-variable]
     struct mt9m114_device *dev = to_mt9m114_sensor(sd);
                            ^~~
   cc1: all warnings being treated as errors

vim +/dev +818 drivers/staging/media/atomisp/i2c/mt9m114.c

a49d2536 Alan Cox 2017-02-17  812  
a49d2536 Alan Cox 2017-02-17  813  static int mt9m114_get_fmt(struct v4l2_subdev *sd,
a49d2536 Alan Cox 2017-02-17  814  				struct v4l2_subdev_pad_config *cfg,
a49d2536 Alan Cox 2017-02-17  815  				struct v4l2_subdev_format *format)
a49d2536 Alan Cox 2017-02-17  816  {
a49d2536 Alan Cox 2017-02-17  817      struct v4l2_mbus_framefmt *fmt = &format->format;
a49d2536 Alan Cox 2017-02-17 @818  	struct mt9m114_device *dev = to_mt9m114_sensor(sd);
a49d2536 Alan Cox 2017-02-17  819  	int width, height;
a49d2536 Alan Cox 2017-02-17  820  	int ret;
a49d2536 Alan Cox 2017-02-17  821  	if (format->pad)

:::::: The code at line 818 was first introduced by commit
:::::: a49d25364dfb9f8a64037488a39ab1f56c5fa419 staging/atomisp: Add support for the Intel IPU v2

:::::: TO: Alan Cox <alan@linux.intel.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Arnd Bergmann March 28, 2017, 8:40 p.m. UTC | #2
On Tue, Mar 28, 2017 at 10:10 PM, kbuild test robot <lkp@intel.com> wrote:
> Hi Arnd,
>
> [auto build test ERROR on staging/staging-testing]
> [also build test ERROR on next-20170328]
> [cannot apply to linuxtv-media/master v4.11-rc4]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/staging-atomisp-avoid-false-positive-maybe-uninitialized-warning/20170329-023715
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>    drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_get_fmt':
>>> drivers/staging/media/atomisp/i2c/mt9m114.c:818:25: error: unused variable 'dev' [-Werror=unused-variable]
>      struct mt9m114_device *dev = to_mt9m114_sensor(sd);
>                             ^~~
>    drivers/staging/media/atomisp/i2c/mt9m114.c: In function 'mt9m114_s_exposure_selection':
>    drivers/staging/media/atomisp/i2c/mt9m114.c:1179:25: error: unused variable 'dev' [-Werror=unused-variable]
>      struct mt9m114_device *dev = to_mt9m114_sensor(sd);
>                             ^~~
>    cc1: all warnings being treated as errors
>

Very odd, I should obviouly have run into these when verifying my patch.
I guess

I'm preparing an v2 patch and will send that once I've built some more
randconfigs on top of recreating the bug and fix.

     Arnd
diff mbox

Patch

diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c b/drivers/staging/media/atomisp/i2c/mt9m114.c
index c4f4c888a59a..12b19fbafb5c 100644
--- a/drivers/staging/media/atomisp/i2c/mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
@@ -689,12 +689,13 @@  static struct mt9m114_res_struct *mt9m114_to_res(u32 w, u32 h)
 	return &mt9m114_res[index];
 }
 
-static int mt9m114_res2size(unsigned int res, int *h_size, int *v_size)
+static int mt9m114_res2size(struct v4l2_subdev *sd, int *h_size, int *v_size)
 {
+	struct mt9m114_device *dev = to_mt9m114_sensor(sd);
 	unsigned short hsize;
 	unsigned short vsize;
 
-	switch (res) {
+	switch (dev->res) {
 	case MT9M114_RES_736P:
 		hsize = MT9M114_RES_736P_SIZE_H;
 		vsize = MT9M114_RES_736P_SIZE_V;
@@ -708,7 +709,8 @@  static int mt9m114_res2size(unsigned int res, int *h_size, int *v_size)
 		vsize = MT9M114_RES_960P_SIZE_V;
 		break;
 	default:
-		WARN(1, "%s: Resolution 0x%08x unknown\n", __func__, res);
+		v4l2_err(sd, "%s: Resolution 0x%08x unknown\n", __func__,
+			 dev->res);
 		return -EINVAL;
 	}
 
@@ -820,7 +822,7 @@  static int mt9m114_get_fmt(struct v4l2_subdev *sd,
 		return -EINVAL;
 	fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
 
-	ret = mt9m114_res2size(dev->res, &width, &height);
+	ret = mt9m114_res2size(sd, &width, &height);
 	if (ret)
 		return ret;
 	fmt->width = width;
@@ -1192,7 +1194,7 @@  static int mt9m114_s_exposure_selection(struct v4l2_subdev *sd,
 	grid_right = sel->r.left + sel->r.width - 1;
 	grid_bottom = sel->r.top + sel->r.height - 1;
 
-	ret = mt9m114_res2size(dev->res, &width, &height);
+	ret = mt9m114_res2size(sd, &width, &height);
 	if (ret)
 		return ret;