diff mbox series

[1/7,media] pwc: hide unused label

Message ID 1453817424-3080054-1-git-send-email-arnd@arndb.de
State New
Headers show
Series [1/7,media] pwc: hide unused label | expand

Commit Message

Arnd Bergmann Jan. 26, 2016, 2:09 p.m. UTC 
The pwc driver causes a warning when CONFIG_USB_PWC_INPUT_EVDEV is unset:

drivers/media/usb/pwc/pwc-if.c: In function 'usb_pwc_probe':
drivers/media/usb/pwc/pwc-if.c:1115:1: warning: label 'err_video_unreg' defined but not used [-Wunused-label]

Obviously, the cleanup of &pdev->vdev is not needed without the input device,
so we can just move it inside of the existing #ifdef and remove the
extra label.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/media/usb/pwc/pwc-if.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
2.7.0

Comments

kernel test robot Jan. 26, 2016, 2:40 p.m. UTC | #1 
Hi Arnd,

[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on v4.5-rc1 next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Arnd-Bergmann/pwc-hide-unused-label/20160126-221727
base:   git://linuxtv.org/media_tree.git master
config: xtensa-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=xtensa 

All errors (new ones prefixed by >>):

   drivers/media/usb/pwc/pwc-if.c: In function 'usb_pwc_probe':
>> drivers/media/usb/pwc/pwc-if.c:1092:3: error: label 'err_video_unreg' used but not defined

      goto err_video_unreg;
      ^

vim +/err_video_unreg +1092 drivers/media/usb/pwc/pwc-if.c

479567ce3 drivers/media/video/pwc/pwc-if.c Hans Verkuil    2010-09-12  1086  
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1087  #ifdef CONFIG_USB_PWC_INPUT_EVDEV
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1088  	/* register webcam snapshot button input device */
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1089  	pdev->button_dev = input_allocate_device();
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1090  	if (!pdev->button_dev) {
89dec01b7 drivers/media/video/pwc/pwc-if.c Dmitry Torokhov 2009-08-14  1091  		rc = -ENOMEM;
89dec01b7 drivers/media/video/pwc/pwc-if.c Dmitry Torokhov 2009-08-14 @1092  		goto err_video_unreg;
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1093  	}
e32a7eccd drivers/media/video/pwc/pwc-if.c Nam Phạm Thành  2009-01-12  1094  
89dec01b7 drivers/media/video/pwc/pwc-if.c Dmitry Torokhov 2009-08-14  1095  	usb_make_path(udev, pdev->button_phys, sizeof(pdev->button_phys));

:::::: The code at line 1092 was first introduced by commit
:::::: 89dec01b7e251697720ac3d6a5310d72359eba69 V4L/DVB (12489): pwc - fix few use-after-free and memory leaks

:::::: TO: Dmitry Torokhov <dmitry.torokhov@gmail.com>
:::::: CC: Mauro Carvalho Chehab <mchehab@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Mauro Carvalho Chehab Jan. 26, 2016, 3:04 p.m. UTC | #2 
Em Tue, 26 Jan 2016 15:10:01 +0100
Arnd Bergmann <arnd@arndb.de> escreveu:

> If MEDIA_SUBDRV_AUTOSELECT and VIDEO_GO7007 are both set, we

> automatically select VIDEO_OV7640, but that depends on MEDIA_CAMERA_SUPPORT,

> so we get a Kconfig warning if that is disabled:

> 

> warning: (VIDEO_GO7007) selects VIDEO_OV7640 which has unmet direct dependencies (MEDIA_SUPPORT && I2C && VIDEO_V4L2 && MEDIA_CAMERA_SUPPORT)

> 

> This adds another dependency so we don't accidentally select

> it when it is unavailable.


This is another bogus warning.

The MEDIA_foo_SUPPORT actually controls what drivers are visible,
but it doesn't enable any driver. They just control the visibility
of the drivers, and the APIs that will be enabled (V4L2 and/or DVB).

The aim of MEDIA_foo_SUPPORT is to make life easier to end users,
making easier for them to filter the drivers that they may need.

If one selects MEDIA_VIDEO_SUPPORT, the V4L2 API (and V4L core) will be 
enabled, and all drivers that support a camera should appear at the 
menu, including drivers that *also* support other features (like TV
and/or stream support).

That's the case of go7007.

That has nothing to do with the features selection for such driver.

Once go7007 driver is selected, if MEDIA_SUBDRV_AUTOSELECT, all
i2c drivers that cope together with go7007 are selected, making the
driver to fully support all devices it knows.

Advanced users may unselect MEDIA_SUBDRV_AUTOSELECT and add there just
the I2C driver(s) it needs for his specific hardware.

Despite the warning, the Kconfig will do the right thing, not
allowing invalid configurations.

Regards,
Mauro


> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/media/usb/go7007/Kconfig | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/media/usb/go7007/Kconfig b/drivers/media/usb/go7007/Kconfig

> index 95a3af644a92..af1d02430931 100644

> --- a/drivers/media/usb/go7007/Kconfig

> +++ b/drivers/media/usb/go7007/Kconfig

> @@ -11,7 +11,7 @@ config VIDEO_GO7007

>  	select VIDEO_TW2804 if MEDIA_SUBDRV_AUTOSELECT

>  	select VIDEO_TW9903 if MEDIA_SUBDRV_AUTOSELECT

>  	select VIDEO_TW9906 if MEDIA_SUBDRV_AUTOSELECT

> -	select VIDEO_OV7640 if MEDIA_SUBDRV_AUTOSELECT

> +	select VIDEO_OV7640 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_CAMERA_SUPPORT

>  	select VIDEO_UDA1342 if MEDIA_SUBDRV_AUTOSELECT

>  	---help---

>  	  This is a video4linux driver for the WIS GO7007 MPEG
Arnd Bergmann Jan. 26, 2016, 3:29 p.m. UTC | #3 
On Tuesday 26 January 2016 15:09:55 Arnd Bergmann wrote:
> The pwc driver causes a warning when CONFIG_USB_PWC_INPUT_EVDEV is unset:

> 

> drivers/media/usb/pwc/pwc-if.c: In function 'usb_pwc_probe':

> drivers/media/usb/pwc/pwc-if.c:1115:1: warning: label 'err_video_unreg' defined but not used [-Wunused-label]

> 

> Obviously, the cleanup of &pdev->vdev is not needed without the input device,

> so we can just move it inside of the existing #ifdef and remove the

> extra label.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> 


Please ignore patch 1. I made some late changes and failed to noticed
the build failure I introduced in another configuration.

	Arnd
Mauro Carvalho Chehab Jan. 26, 2016, 3:41 p.m. UTC | #4 
Em Tue, 26 Jan 2016 13:04:03 -0200
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Tue, 26 Jan 2016 15:10:01 +0100

> Arnd Bergmann <arnd@arndb.de> escreveu:

> 

> > If MEDIA_SUBDRV_AUTOSELECT and VIDEO_GO7007 are both set, we

> > automatically select VIDEO_OV7640, but that depends on MEDIA_CAMERA_SUPPORT,

> > so we get a Kconfig warning if that is disabled:

> > 

> > warning: (VIDEO_GO7007) selects VIDEO_OV7640 which has unmet direct dependencies (MEDIA_SUPPORT && I2C && VIDEO_V4L2 && MEDIA_CAMERA_SUPPORT)

> > 

> > This adds another dependency so we don't accidentally select

> > it when it is unavailable.  

> 

> This is another bogus warning.

> 

> The MEDIA_foo_SUPPORT actually controls what drivers are visible,

> but it doesn't enable any driver. They just control the visibility

> of the drivers, and the APIs that will be enabled (V4L2 and/or DVB).

> 

> The aim of MEDIA_foo_SUPPORT is to make life easier to end users,

> making easier for them to filter the drivers that they may need.

> 

> If one selects MEDIA_VIDEO_SUPPORT, the V4L2 API (and V4L core) will be 

> enabled, and all drivers that support a camera should appear at the 

> menu, including drivers that *also* support other features (like TV

> and/or stream support).

> 

> That's the case of go7007.

> 

> That has nothing to do with the features selection for such driver.

> 

> Once go7007 driver is selected, if MEDIA_SUBDRV_AUTOSELECT, all

> i2c drivers that cope together with go7007 are selected, making the

> driver to fully support all devices it knows.

> 

> Advanced users may unselect MEDIA_SUBDRV_AUTOSELECT and add there just

> the I2C driver(s) it needs for his specific hardware.

> 

> Despite the warning, the Kconfig will do the right thing, not

> allowing invalid configurations.

> 

> Regards,

> Mauro

> 

> 

> > 

> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> > ---

> >  drivers/media/usb/go7007/Kconfig | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > 

> > diff --git a/drivers/media/usb/go7007/Kconfig b/drivers/media/usb/go7007/Kconfig

> > index 95a3af644a92..af1d02430931 100644

> > --- a/drivers/media/usb/go7007/Kconfig

> > +++ b/drivers/media/usb/go7007/Kconfig

> > @@ -11,7 +11,7 @@ config VIDEO_GO7007

> >  	select VIDEO_TW2804 if MEDIA_SUBDRV_AUTOSELECT

> >  	select VIDEO_TW9903 if MEDIA_SUBDRV_AUTOSELECT

> >  	select VIDEO_TW9906 if MEDIA_SUBDRV_AUTOSELECT

> > -	select VIDEO_OV7640 if MEDIA_SUBDRV_AUTOSELECT

> > +	select VIDEO_OV7640 if MEDIA_SUBDRV_AUTOSELECT && MEDIA_CAMERA_SUPPORT


Hmm... on a second thought, it could make sense to only auto-select
the sensor drivers if MEDIA_CAMERA_SUPPORT, and to only auto-select
tuners if MEDIA_*_TV_SUPPORT is selected.

However, in this case, we'll need to dig into all Kconfig autoselect
options in order to make it right.

I guess we should also likely need to document to the user that the
driver won't be fully supported, as we'll be changing the behavior
of the menus.

Comments?

Regards,
Mauro
diff mbox series

Patch

diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 086cf1c7bd7d..bdd416af84c7 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -1106,14 +1106,13 @@  static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
 	if (rc) {
 		input_free_device(pdev->button_dev);
 		pdev->button_dev = NULL;
-		goto err_video_unreg;
+		video_unregister_device(&pdev->vdev);
+		goto err_unregister_v4l2_dev;
 	}
 #endif
 
 	return 0;
 
-err_video_unreg:
-	video_unregister_device(&pdev->vdev);
 err_unregister_v4l2_dev:
 	v4l2_device_unregister(&pdev->v4l2_dev);
 err_free_controls: