Thanks for this and the other feedback.
The concern, without knowing the full history, is if video_device_alloc
changes to do more than just allocate the whole structure with a single
call to kzalloc? Otherwise, why have this extra indirection and
overhead in most V4L drivers?
The majority of V4L drivers are using video_device_alloc. Very few
(bw-qcam.h, c-qcam.c, cpia.h, pvrusb2, usbvideo) are using "struct
video_device" statically similar to solution 1. Three drivers(zoran,
radio-gemtek, saa5249) are allocating their own video_device structure
directly with kzalloc similar to solution #2.
The call definitely needs checked, but I'd like some more feedback on this.
Thanks and best regards,
Dean
David Ellingsworth wrote:
This patch looks good, but there was one other thing that caught my eye.
In s2255_probe_v4l, video_device_alloc is called for each video
device, which is nothing more than a call to kzalloc, but the result
of the call is never verified.
Given that this driver has a fixed number of video device nodes, the
array of video_device structs could be allocated within the s2255_dev
struct. This would remove the extra calls to video_device_alloc,
video_device_release, and the additional error checks that should have
been there. If you'd prefer to keep the array of video_device structs
independent of the s2255_dev struct, an alternative would be to
dynamically allocate the entire array at once using kcalloc and store
only the pointer to the array in the s2255_dev struct. In my opinion,
either of these methods would be better than calling
video_device_alloc for each video device that needs to be registered.
Regards,
David Ellingsworth
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
