diff mbox

video/s255drv: cleanup. remove uneeded NULL check

Message ID 20100328112909.GP5069@bicker (mailing list archive)
State Superseded, archived
Headers

Commit Message

Dan Carpenter March 28, 2010, 11:29 a.m. UTC 
  "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.

Signed-off-by: Dan Carpenter <error27@gmail.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Ellingsworth March 29, 2010, 3:54 p.m. UTC | #1 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/video/s2255drv.c b/drivers/media/video/s2255drv.c
index fb742f1..e70af5d 100644
--- a/drivers/media/video/s2255drv.c
+++ b/drivers/media/video/s2255drv.c
@@ -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 = {