Gerd Knorr wrote:
>It also provides a ioctl wrapper function which handles copying the
>ioctl args from/to userspace, so we have this at one place can drop all
>the copy_from/to_user calls within the v4l device driver ioctl handlers.
>
Excellent work. I have no complaints, just a few questions:
1. Would it be better to memset the temp buffer in video_generic_ioctl()
rather than in the driver? I've seen so many drivers forget to do this,
and it's a potential (albeit very small) security hole.
2. In skeleton_open(), couldn't the device_data lookup code be replaced
with:
struct video_device *vdev = video_devdata(file);
struct device_data *dev = vdev->priv;
3. In skeleton_initdev(), shouldn't...
dev->vdev = skeleton_template;
...be...
memcpy(&dev->vdev, &skeleton_template, sizeof(skeleton_template);
4. Is it safe to keep even 128 bytes on the stack in
video_generic_ioctl()? Consider that devices might spend a relatively
long time blocking on VIDIOCSYNC. With 32 devices in use at once, you'd
be coming dangerously close to a stack overflow. IMHO it would be better
to only allocate as much as MCAPTURE and SYNC need, and fall back on
kmalloc for the less time-critical ones (if necessary).
Other than that, I extremely happy with what you've done!
-- Mark McClelland mmcclell@bigfoot.com- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Feb 15 2002 - 21:00:30 EST