On Wednesday 18 November 2009, Andi Kleen wrote:
> > Doing all these out-of-line hidden over here in a separate file just
> > seems crazy,
> > and I'm having trouble wondering why ppl consider it a better idea at all.
>
> The separate file is on the way out, the modern way is to use
> ->compat_ioctl in the driver itself. Near all drivers except
> for a few exceptions like DRM put it into the same file.
> I think you should just move the code around in DRM too
> and drop the separate file.
Right. Commonly, you can simply handle the incompatible stuff in
->compat_ioctl and then call the regular function. Another way to
do it if you want to consolidate even more is to pass a flag down
the actual function and do
static int my_common_ioctl(struct my_object *my, unsigned int cmd, unsigned
long arg,
void __user *argp, int compat)
{
...
}
static long my_native_ioctl(struct file *file, unsigned int cmd, unsigned long
arg)
{
return my_commmon_ioctl(file->private_data, cmd, arg, (void __user
*)arg, 0);
}
static long my_compat_ioctl(struct file *file, unsigned int cmd, unsigned long
arg)
{
return my_commmon_ioctl(file->private_data, cmd, arg, compat_ptr(arg),
1);
}
Doing that, you can have the ioctl handling for a subsystem consolidated in one
place, just doing the copy_from_user() part separately.
For the specific case of drm, where you have a table with all the ioctls, I can
see yet another option: add another function pointer to 'struct drm_ioctl_desc'
that does the conversion, e.g.
int drm_version_compat(void __user *argp, void *kdata, int dir)
{
struct compat_drm_version __user *uversion = argp;
struct compat_drm_version cversion;
struct drm_version *version = kdata;
if (dir == IOC_IN) {
if (copy_from_user(&cversion, uversion, sizeof(cversion);
return -EFAULT;
*version = (struct drm_version) {
.version_major = cversion.version_major,
.version_minor = cversion.version_minor,
.version_mpatchlevel = cversion.version_patchlevel,
.name_len = cversion.name_len,
.name = compat_ptr(cversion.name),
.date_len = cversion.date_len,
.date = compat_ptr(cversion.date),
.desc_len = cversion.desc_len,
.desc = compat_ptr(cversion.desc),
};
return 0;
} else if (dir == IOC_OUT) {
cversion = (struct drm_version) {
.version_major = version->version_major,
.version_minor = version->version_minor,
.version_mpatchlevel = version->version_patchlevel,
.name_len = version->name_len,
.name = (unsigned long)version->name,
.date_len = version->date_len,
.date = (unsigned long)version->date,
.desc_len = version->desc_len,
.desc = (unsigned long)version->desc,
};
if (copy_to_user(uversion, &cversion, sizeof(cversion);
return -EFAULT;
return 0;
}
return -EINVAL;
}
static struct drm_ioctl_desc drm_ioctls[] = {
DRM_IOCTL_DEF(DRM_IOCTL_VERSION, drm_version, drm_version_compat, 0),
...
};
Arnd <><
------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day
trial. Simplify your report design, integration and deployment - and focus on
what you do best, core application coding. Discover what's new with
Crystal Reports now. http://p.sf.net/sfu/bobj-july
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
