On Thu, Nov 17, 2011 at 11:07:16AM +0100, Jean-Francois Moine wrote:
> 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?
>
Hi Jef,
I just sent a patch to linux-media for this little issue.
I realize it is only a very minor patch,
so I am not sure If I am helping or just annoying the developers ;)
Anyway, if you could check the patch I would appreciate it.
A few questions arose:
* I made the patch on this tree: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media
not sure if this is ok.
* Should I send gspca's patches to anyone else besides you and the list?
* I have this on my MAINTANIERS file: git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-2.6.git
But that repo seems no longer alive, maybe MAINTAINERS need some
updating.
Again, hope the patch helps,
Thanks,
Ezequiel.
--
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
