[PATCH 3/4] VFS: factor out three helpers for FIBMAP/FIONBIO/FIOASYNC file ioctls

2007-10-30 Thread Erez Zadok
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/ioctl.c |  129 +++-
 1 files changed, 75 insertions(+), 54 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 1ab7b7d..cd8c1a3 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -53,32 +53,34 @@ long vfs_ioctl(struct file *filp, unsigned int cmd,
 }
 EXPORT_SYMBOL_GPL(vfs_ioctl);
 
+static int ioctl_fibmap(struct file *filp, int __user *p)
+{
+   struct address_space *mapping = filp->f_mapping;
+   int res, block;
+
+   /* do we support this mess? */
+   if (!mapping->a_ops->bmap)
+   return -EINVAL;
+   if (!capable(CAP_SYS_RAWIO))
+   return -EPERM;
+   res = get_user(block, p);
+   if (res)
+   return res;
+   lock_kernel();
+   res = mapping->a_ops->bmap(mapping, block);
+   unlock_kernel();
+   return put_user(res, p);
+}
+
 static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
 {
-   int error;
-   int block;
struct inode *inode = filp->f_path.dentry->d_inode;
int __user *p = (int __user *)arg;
 
switch (cmd) {
case FIBMAP:
-   {
-   struct address_space *mapping = filp->f_mapping;
-   int res;
-   /* do we support this mess? */
-   if (!mapping->a_ops->bmap)
-   return -EINVAL;
-   if (!capable(CAP_SYS_RAWIO))
-   return -EPERM;
-   error = get_user(block, p);
-   if (error)
-   return error;
-   lock_kernel();
-   res = mapping->a_ops->bmap(mapping, block);
-   unlock_kernel();
-   return put_user(res, p);
-   }
+   return ioctl_fibmap(filp, p);
case FIGETBSZ:
return put_user(inode->i_sb->s_blocksize, p);
case FIONREAD:
@@ -88,6 +90,57 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
return vfs_ioctl(filp, cmd, arg);
 }
 
+static int ioctl_fionbio(struct file *filp, int __user *argp)
+{
+   unsigned int flag;
+   int on, error;
+
+   error = get_user(on, argp);
+   if (error)
+   return error;
+   flag = O_NONBLOCK;
+#ifdef __sparc__
+   /* SunOS compatibility item. */
+   if (O_NONBLOCK != O_NDELAY)
+   flag |= O_NDELAY;
+#endif
+   if (on)
+   filp->f_flags |= flag;
+   else
+   filp->f_flags &= ~flag;
+   return error;
+}
+
+static int ioctl_fioasync(unsigned int fd, struct file *filp,
+ int __user *argp)
+{
+   unsigned int flag;
+   int on, error;
+
+   error = get_user(on, argp);
+   if (error)
+   return error;
+   flag = on ? FASYNC : 0;
+
+   /* Did FASYNC state change ? */
+   if ((flag ^ filp->f_flags) & FASYNC) {
+   if (filp->f_op && filp->f_op->fasync) {
+   lock_kernel();
+   error = filp->f_op->fasync(fd, filp, on);
+   unlock_kernel();
+   } else
+   error = -ENOTTY;
+   }
+   if (error)
+   return error;
+
+   if (on)
+   filp->f_flags |= FASYNC;
+   else
+   filp->f_flags &= ~FASYNC;
+   return error;
+}
+
 /*
  * When you add any new common ioctls to the switches above and below
  * please update compat_sys_ioctl() too.
@@ -98,8 +151,8 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
 int do_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 unsigned long arg)
 {
-   unsigned int flag;
-   int on, error = 0;
+   int error = 0;
+   int __user *argp = (int __user *)arg;
 
switch (cmd) {
case FIOCLEX:
@@ -111,43 +164,11 @@ int do_ioctl(struct file *filp, unsigned int fd, unsigned 
int cmd,
break;
 
case FIONBIO:
-   error = get_user(on, (int __user *)arg);
-   if (error)
-   break;
-   flag = O_NONBLOCK;
-#ifdef __sparc__
-   /* SunOS compatibility item. */
-   if (O_NONBLOCK != O_NDELAY)
-   flag |= O_NDELAY;
-#endif
-   if (on)
-   filp->f_flags |= flag;
-   else
-   filp->f_flags &= ~flag;
+   error = ioctl_fionbio(filp, argp);
break;
 
case FIOASYNC:
-   error = get_user(on, (int __user *)arg);
-   if (error)
-   break;
-   flag = on ? FASYNC : 0;
-
-   /* Did FASYNC state change ? */
-   if ((flag ^ filp->f_flags) & FASYNC) {
-   if (filp->f_op && filp->f_op->fasync) {
-   lock_kernel();
-   error = filp->f_o

Re: [PATCH 3/4] VFS: factor out three helpers for FIBMAP/FIONBIO/FIOASYNC file ioctls

2007-10-30 Thread Christoph Hellwig
On Tue, Oct 30, 2007 at 01:49:48PM -0400, Erez Zadok wrote:
> BTW, what's the origin of this oddity in fs/ioctl.c:
> 
> #ifdef __sparc__
>   /* SunOS compatibility item. */
>   if (O_NONBLOCK != O_NDELAY)
>   flag |= O_NDELAY;
> #endif
> 
> It seems rather odd to have architecture-specific code in the VFS, no?

When Dave did the sparc port he followed sunos ABIs for lots of things.
When these ABIs are hidden behind ioctl arguments they can't easily
be handled in arch code and we need warts like that.  It's definitively
not recommended for new ports..

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] VFS: factor out three helpers for FIBMAP/FIONBIO/FIOASYNC file ioctls

2007-10-30 Thread Erez Zadok
BTW, what's the origin of this oddity in fs/ioctl.c:

#ifdef __sparc__
/* SunOS compatibility item. */
if (O_NONBLOCK != O_NDELAY)
flag |= O_NDELAY;
#endif

It seems rather odd to have architecture-specific code in the VFS, no?

Erez.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/4] VFS: factor out three helpers for FIBMAP/FIONBIO/FIOASYNC file ioctls

2007-10-30 Thread Christoph Hellwig
> +static int __ioctl_fibmap(struct file *filp, int __user *p)

I'd say kill the __ prefix for all the functions you're adding.

> +static int __ioctl_fionbio(struct file *filp, unsigned long arg)

> +static int __ioctl_fioasync(unsigned int fd, struct file *filp,
> + unsigned long arg)

For these two I'd add a

void __user *argp = (void __user *)arg;

in the caller and then just pass argp down.  That way we have the cast in
one place.

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/4] VFS: factor out three helpers for FIBMAP/FIONBIO/FIOASYNC file ioctls

2007-10-28 Thread Erez Zadok
Signed-off-by: Erez Zadok <[EMAIL PROTECTED]>
---
 fs/ioctl.c |  128 ++-
 1 files changed, 74 insertions(+), 54 deletions(-)

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 34e3f58..8dd2ef1 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -54,32 +54,34 @@ long vfs_ioctl(struct file *filp, unsigned int cmd,
 }
 EXPORT_SYMBOL_GPL(vfs_ioctl);
 
+static int __ioctl_fibmap(struct file *filp, int __user *p)
+{
+   struct address_space *mapping = filp->f_mapping;
+   int res, block;
+
+   /* do we support this mess? */
+   if (!mapping->a_ops->bmap)
+   return -EINVAL;
+   if (!capable(CAP_SYS_RAWIO))
+   return -EPERM;
+   res = get_user(block, p);
+   if (res)
+   return res;
+   lock_kernel();
+   res = mapping->a_ops->bmap(mapping, block);
+   unlock_kernel();
+   return put_user(res, p);
+}
+
 static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
 {
-   int error;
-   int block;
struct inode *inode = filp->f_path.dentry->d_inode;
int __user *p = (int __user *)arg;
 
switch (cmd) {
case FIBMAP:
-   {
-   struct address_space *mapping = filp->f_mapping;
-   int res;
-   /* do we support this mess? */
-   if (!mapping->a_ops->bmap)
-   return -EINVAL;
-   if (!capable(CAP_SYS_RAWIO))
-   return -EPERM;
-   error = get_user(block, p);
-   if (error)
-   return error;
-   lock_kernel();
-   res = mapping->a_ops->bmap(mapping, block);
-   unlock_kernel();
-   return put_user(res, p);
-   }
+   return __ioctl_fibmap(filp, p);
case FIGETBSZ:
return put_user(inode->i_sb->s_blocksize, p);
case FIONREAD:
@@ -89,6 +91,57 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
return vfs_ioctl(filp, cmd, arg);
 }
 
+static int __ioctl_fionbio(struct file *filp, unsigned long arg)
+{
+   unsigned int flag;
+   int on, error;
+
+   error = get_user(on, (int __user *)arg);
+   if (error)
+   return error;
+   flag = O_NONBLOCK;
+#ifdef __sparc__
+   /* SunOS compatibility item. */
+   if (O_NONBLOCK != O_NDELAY)
+   flag |= O_NDELAY;
+#endif
+   if (on)
+   filp->f_flags |= flag;
+   else
+   filp->f_flags &= ~flag;
+   return error;
+}
+
+static int __ioctl_fioasync(unsigned int fd, struct file *filp,
+   unsigned long arg)
+{
+   unsigned int flag;
+   int on, error;
+
+   error = get_user(on, (int __user *)arg);
+   if (error)
+   return error;
+   flag = on ? FASYNC : 0;
+
+   /* Did FASYNC state change ? */
+   if ((flag ^ filp->f_flags) & FASYNC) {
+   if (filp->f_op && filp->f_op->fasync) {
+   lock_kernel();
+   error = filp->f_op->fasync(fd, filp, on);
+   unlock_kernel();
+   } else
+   error = -ENOTTY;
+   }
+   if (error)
+   return error;
+
+   if (on)
+   filp->f_flags |= FASYNC;
+   else
+   filp->f_flags &= ~FASYNC;
+   return error;
+}
+
 /*
  * When you add any new common ioctls to the switches above and below
  * please update compat_sys_ioctl() too.
@@ -99,8 +152,7 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
 int do_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
 unsigned long arg)
 {
-   unsigned int flag;
-   int on, error = 0;
+   int error = 0;
 
switch (cmd) {
case FIOCLEX:
@@ -112,43 +164,11 @@ int do_ioctl(struct file *filp, unsigned int fd, unsigned 
int cmd,
break;
 
case FIONBIO:
-   error = get_user(on, (int __user *)arg);
-   if (error)
-   break;
-   flag = O_NONBLOCK;
-#ifdef __sparc__
-   /* SunOS compatibility item. */
-   if (O_NONBLOCK != O_NDELAY)
-   flag |= O_NDELAY;
-#endif
-   if (on)
-   filp->f_flags |= flag;
-   else
-   filp->f_flags &= ~flag;
+   error = __ioctl_fionbio(filp, arg);
break;
 
case FIOASYNC:
-   error = get_user(on, (int __user *)arg);
-   if (error)
-   break;
-   flag = on ? FASYNC : 0;
-
-   /* Did FASYNC state change ? */
-   if ((flag ^ filp->f_flags) & FASYNC) {
-   if (filp->f_op && filp->f_op->fasync) {
-   lock_kernel();
-   error = filp->f_op->fasyn