lkml.org 
[lkml]   [2008]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] ioctl: generic ioctl dispatcher
Andi Kleen wrote:
> Avi Kivity <avi@redhat.com> writes:
>
>
>> +long dispatch_ioctl(struct inode *inode, struct file *filp,
>> + unsigned cmd, unsigned long arg,
>> + const struct ioctl_handler *handlers,
>> + long (*fallback)(const struct ioctl_arg *arg))
>>
>
> The basic idea is good, but i don't like the proliferation of callbacks,
> which tends to make complicated code and is ugly for simple code
> (which a lot of ioctls are)
>
>

If the simple calls mostly don't use the argument as a pointer, they are
better off using a plain switch. For my own code, I usually leave the
boilerplate within the switch and the app-specific code in a separate
function anyway, so there's no big change in style.

The main motivation here was the extensibility (patch 2), which becomes
much more difficult with a switch.

> How about you make it return an number that can index a switch() instead?
> Then everything could be still kept in the same function.
>
>

We need to execute code both before and after the handler, so it would
look pretty ugly:

long my_ioctl_handler(...)
{
struct ioctl_arg iarg;
...
long ret;

ret = dispatch_ioctl_begin(&iarg, ...);
if (ret < 0)
return ret;
switch (ret) {
case _IOC_KEY(MY_IOCTL):
// your stuff goes here
break;
...
}
dispatch_ioctl_end(&iarg, ret);
return ret;
}

The only clean way to do this without callbacks is with
constructors/destructors, but we don't have those in C.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.



\
 
 \ /
  Last update: 2008-09-30 11:11    [W:0.051 / U:0.724 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site