Re: [PATCH 2/9] PM: suspend_block: Add driver to access suspend blockers from user-space

[Pavel Machek <pavel@ucw.cz>]

On Wed 2009-05-06 18:31:36, Arve Hj?nnev?g wrote:
> On Tue, May 5, 2009 at 1:16 PM, Pavel Machek <pavel@ucw.cz> wrote:
> > On Tue 2009-05-05 21:18:42, Arve Hj??nnev??g wrote:
> >> Add a misc device, "suspend_blocker", that allows user-space processes
> >> to block auto suspend. The device has ioctls to create a suspend_blocker,
> >> and to block and unblock suspend. To delete the suspend_blocker, close
> >> the device.
> >>
> >
> >> +static int create_user_suspend_blocker(struct file *file, void __user *name,
> >> +                              size_t name_len)
> >> +{
> >> +     struct user_suspend_blocker *bl;
> >> +     if (file->private_data)
> >> +             return -EBUSY;
> >> +     bl = kzalloc(sizeof(*bl) + name_len + 1, GFP_KERNEL);
> > ...
> >> +static long user_suspend_blocker_ioctl(struct file *file, unsigned int cmd,
> >> +                             unsigned long _arg)
> >> +{
> >> +     void __user *arg = (void __user *)_arg;
> >> +     struct user_suspend_blocker *bl;
> >> +     long ret;
> >> +
> >> +     mutex_lock(&ioctl_lock);
> >> +     if ((cmd & ~IOCSIZE_MASK) == SUSPEND_BLOCKER_IOCTL_INIT(0)) {
> >> +             ret = create_user_suspend_blocker(file, arg, _IOC_SIZE(cmd));
> >> +             goto done;
> >> +     }
> >
> > Wait a moment, wtf is this? Not one ioctl but one ioctl per length of
> > string?!
> 
> This is not uncommon. _IOC encodes the size of the argument, and if
> this is not a fixed size, then the raw ioctl number change based on
> the size passed in. Look at input.h is you want other examples of
> this.

Ok, I found few examples in code:

pavel@amd:/data/l/linux$ grep -ri IOCSIZE_MASK .
./arch/alpha/include/asm/ioctl.h:#define IOCSIZE_MASK	(_IOC_SIZEMASK
<< _IOC_SIZESHIFT)
./arch/mips/include/asm/ioctl.h:#define IOCSIZE_MASK	(_IOC_SIZEMASK
<< _IOC_SIZESHIFT)
./arch/sparc/include/asm/ioctl.h:#define IOCSIZE_MASK
(_IOC_XSIZEMASK << _IOC_SIZESHIFT)
./drivers/input/joydev.c:		if ((cmd & ~IOCSIZE_MASK) ==
JSIOCGNAME(0)) {
./drivers/message/fusion/mptctl.c:	if ((cmd & ~IOCSIZE_MASK) ==
(MPTIOCINFO & ~IOCSIZE_MASK)) {
./drivers/message/fusion/mptctl.c:	else if ((cmd & ~IOCSIZE_MASK)
== (HP_GETHOSTINFO & ~IOCSIZE_MASK))
./drivers/mtd/mtdchar.c:	size = (cmd & IOCSIZE_MASK) >>
IOCSIZE_SHIFT;
./drivers/pcmcia/pcmcia_ioctl.c:    size = (cmd & IOCSIZE_MASK) >>
IOCSIZE_SHIFT;
./include/asm-generic/ioctl.h:#define IOCSIZE_MASK	(_IOC_SIZEMASK
<< _IOC_SIZESHIFT)

I still do not like introducing 16000 ioctls, and I'm not sure how our
ioctl compatibility layer playes with this.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html