On Wed, 16 Nov 2011 15:19:04 -0300
Ezequiel García <elezegarcia@xxxxxxxxx> wrote:
> In 'media/video/gspca/gspca.c' I really hated this cast (maybe because
> I am too dumb to understand it):
>
> gspca_dev = (struct gspca_dev *) video_devdata(file);
>
> wich is only legal because a struct video_device is the first member
> of gspca_dev. IMHO, this is 'unnecesary obfuscation'.
> The thing is the driver is surely working fine and there is no good
> reasong for the change.
>
> Is it ok to submit a patchset to change this? Something like this:
>
> diff --git a/drivers/media/video/gspca/gspca.c
> b/drivers/media/video/gspca/gspca.c
> index 881e04c..5d962ce 100644
> --- a/drivers/media/video/gspca/gspca.c
> +++ b/drivers/media/video/gspca/gspca.c
> @@ -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)
Hi Ezequiel,
You are right, the cast is not a good way (and there are a lot of them
in the gspca subdrivers), but your patch does not work because the
'private_data' of the device is not initialized (there is no call to
video_set_drvdata).
So, a possible cleanup could be:
> - gspca_dev = (struct gspca_dev *) video_devdata(file);
> + gspca_dev = container_of(video_devdata(file), struct gspca_dev, vdev);
Is it OK for you?
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
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
