video/s255drv: cleanup. remove uneeded NULL check
Subsystems (2)
PATCH 1/1 video/s255drv: cleanup. remove uneeded NULL check
"dev" can never be NULL there so there is no need to check it. Also I made a couple of white space changes to the declaration of "dev".
This eliminates a smatch warning about checking for NULL after a dereference.
drivers/media/video/s2255drv.c +5 -6
@@ -2582,8 +2582,9 @@ error:/* disconnect routine. when board is removed physically or with rmmod */ static void s2255_disconnect (struct usb_interface *interface) { -struct s2255_dev *dev =NULL ; +struct s2255_dev *dev;int i; +dprintk (1 ,"s2255: disconnect interface %p\n" , interface); dev =usb_get_intfdata (interface);
@@ -2602,11 +2603,9 @@ static void s2255_disconnect(struct usb_interface *interface)usb_set_intfdata (interface,NULL );mutex_unlock (&dev->open_lock); -if (dev) { -kref_put (&dev->kref, s2255_destroy); -dprintk (1 ,"s2255drv: disconnect\n" ); -dev_info (&interface->dev,"s2255usb now disconnected\n" ); - } +kref_put (&dev->kref, s2255_destroy); +dprintk (1 ,"s2255drv: disconnect\n" ); +dev_info (&interface->dev,"s2255usb now disconnected\n" ); }static struct usb_driver s2255_driver = {
I don't have any problems with this patch in particular but would like to note that this driver could use a little refactoring.
One thing I noticed just while glancing at the code is that the return value from s2255_probe_v4l is not checked in s2255_probe. As a result the driver could fail to register some or all of it's video device nodes and still continue to load.
Also the use of kref, while needed in this driver due to the number of video nodes created, is probably a bit overused. In my opinion video_unregister_device should be called in the usb disconnect callback for each registered video device. This ensures that no future calls to open will occur for that device. Subsequently, once all applications have stopped using the previously registered video_device, the release callback of the video_device struct will fire. Therefore if the device kref is incremented for each registered video device (during probe) and then properly decremented during the video_device release callback for each device the device driver's structure may then be freed. This approach should lead to a much cleaner implementation of the open, release, and disconnect callbacks.
Regards,
David Ellingsworth