On Tuesday 30 March 2010 22:49:08 dean wrote:
> 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?
It is unlikely that video_device_alloc() will change. I think it is just
historical. I have a preference for not allocating it at all, but embedding
it in a larger struct. This type of embedding is very common in the kernel.
But if you do allocate it, then use video_device_alloc() rather than kzalloc,
if only because that makes it consistent and easier to grep on should we
need to replace it in the future.
Regards,
Hans
> 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 [email protected]
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
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
