Re: [linuxtv-commits] [hg:v4l-dvb] Remove unused inode parameter from video_ioctl2

[Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx>]

On Tue, 21 Oct 2008 20:08:26 +0200
Hans Verkuil <hverkuil@xxxxxxxxx> wrote:

> On Tuesday 21 October 2008 19:32:37 Mauro Carvalho Chehab wrote:
> > On Tue, 21 Oct 2008 18:50:20 +0200
> >
> > Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> > > Hi Mauro,
> > >
> > > Good move this change, but why keep the name '__video_ioctl2'
> > > instead of renaming it to 'video_ioctl2_unlocked'?
> > >
> > > '__video_ioctl2' is not exactly a very descriptive name IMHO :-)
> >
> > It took me some time to understand video_ioctl2_unlocked(), since I
> > didn't see any lock anywhere :)
> >
> > After understanding that it were just getting hid of inode, I decided
> > to remove inode from the ioctl handlers, keeping it only at the
> > fops.ioctl handler.
> >
> > I've named as __video_ioctl2 for two reasons:
> >
> > 1) There's no lock at video_ioctl2 or at __video_ioctl2. So, calling
> > it as unlocked seemed a very bad name;
> >
> > 2) It is a kind of standard inside kernel that __foo() being a lower
> > level version of foo().
> 
> Hm, I don't agree with that. If you develop a driver then it is logical 
> to have the choice between:
> 
> .ioctl = video_ioctl2;
> 
> and
> 
> .unlocked_ioctl = video_unlocked_ioctl2;
> 
> (even better than video_ioctl2_unlocked).
> 
> Writing
> 
> .unlocked_ioctl = __video_ioctl2;
> 
> just looks really weird and illogical. It's also not obvious at all that 
> __video_ioctl2 is the version to use for .unlocked_ioctl.
> 
> > Anyway, I hope people will start migrating to the unlocked version.
> > After having everything using __video_ioctl2, we may simple rename it
> > to video_ioctl2.
> 
> Or v4l2_ioctl, which is nicer IMHO.

True.

Maybe we can just rename __video_ioctl2 to video_ioctl and video_ioctl2 to
video_ioctl2_bkl (or a similar name).
> 
> Regards,
> 
> 	Hans




Cheers,
Mauro

_______________________________________________
v4l-dvb-maintainer mailing list
v4l-dvb-maintainer@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer