gspca: replaced static allocation by video_device_alloc/video_device_release

Pushed video_device initialization into a separate function. Replaced static allocation of struct video_device by video_device_alloc/video_device_release usage.

Signed-off-by Ezequiel Garcia<[email protected]>

---

drivers/media/video/gspca/gspca.c +9 -6

index 881e04c..1f27f05 100644
@@ -1292,10 +1292,12 @@ static int vidioc_enum_frameintervals(struct file *filp, void *priv,
 
  static void gspca_release(struct video_device *vfd)
  {
-	struct gspca_dev *gspca_dev = container_of(vfd, struct gspca_dev, vdev);
+	struct gspca_dev *gspca_dev = video_get_drvdata(vfd);
 
  	PDEBUG(D_PROBE, "%s released",
-		video_device_node_name(&gspca_dev->vdev));
+		video_device_node_name(gspca_dev->vdev));
+
+	video_device_release(vfd);
 
  	kfree(gspca_dev->usb_buf);
  	kfree(gspca_dev);

@@ -1304,9 +1306,11 @@ static void gspca_release(struct video_device *vfd)
  static int dev_open(struct file *file)
  {
  	struct gspca_dev *gspca_dev;
+	struct video_device *vdev;
 
  	PDEBUG(D_STREAM, "[%s] open", current->comm);
-	gspca_dev = (struct gspca_dev *) video_devdata(file);
+	vdev = video_devdata(file);
+	gspca_dev = video_get_drvdata(vdev);
  	if (!gspca_dev->present)
  		return -ENODEV;
 

@@ -1318,10 +1322,10 @@ static int dev_open(struct file *file)
  #ifdef GSPCA_DEBUG
  	/* activate the v4l2 debug */
  	if (gspca_debug&  D_V4L2)
-		gspca_dev->vdev.debug |= V4L2_DEBUG_IOCTL
+		gspca_dev->vdev->debug |= V4L2_DEBUG_IOCTL
  					| V4L2_DEBUG_IOCTL_ARG;
  	else
-		gspca_dev->vdev.debug&= ~(V4L2_DEBUG_IOCTL
+		gspca_dev->vdev->debug&= ~(V4L2_DEBUG_IOCTL
  					| V4L2_DEBUG_IOCTL_ARG);
--

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

Hi,

Quoted 3 lines · Ezequiel

Pushed video_device initialization into a separate function. Replaced static allocation of struct video_device by video_device_alloc/video_device_release usage.

NACK! I see a video_device_release call here, but not a video_device_alloc, also you're messing with quite sensitive code here (because a usb device can be unplugged at any time, including when the /dev/video node is open by a process), and changing it from static to dynamic allocation my have more consequences then you see at first (I did not analyze all the code paths for the proposed change, since the last time I audited them for the current static allocation of the videodevice struct code took me hours).

Also static allocation (as part of the driver struct) in general is better then dynamic as it needs less code and helps avoiding memory fragmentation.

All in all I cannot help but feel that you're diving into a piece of code with some drive by shooting style patch without knowing the code in question at all, please don't do that!

Regards,

Hans