Christoph Hellwig <hch@xxxxxx> writes:
> The three subcases of xfs_ioc_xattr don't share any semantics and almost
> no code, so split it into three separate helpers.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Looks good to me, aren't the likely() unlinkely() deprecated ? shouldn't
they be killed ?
Cheers,
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl.c 2008-03-04
> 18:14:57.000000000 +0100
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl.c 2008-03-04
> 18:25:51.000000000 +0100
> @@ -871,85 +871,85 @@ xfs_ioc_fsgetxattr(
> }
>
> STATIC int
> -xfs_ioc_xattr(
> +xfs_ioc_fssetxattr(
> xfs_inode_t *ip,
> struct file *filp,
> - unsigned int cmd,
> void __user *arg)
> {
> struct fsxattr fa;
> struct bhv_vattr *vattr;
> - int error = 0;
> + int error;
> int attr_flags;
> - unsigned int flags;
> +
> + if (copy_from_user(&fa, arg, sizeof(fa)))
> + return -EFAULT;
>
> vattr = kmalloc(sizeof(*vattr), GFP_KERNEL);
> if (unlikely(!vattr))
> return -ENOMEM;
>
> - switch (cmd) {
> - case XFS_IOC_FSSETXATTR: {
> - if (copy_from_user(&fa, arg, sizeof(fa))) {
> - error = -EFAULT;
> - break;
> - }
> -
> - attr_flags = 0;
> - if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> - attr_flags |= ATTR_NONBLOCK;
> -
> - vattr->va_mask = XFS_AT_XFLAGS | XFS_AT_EXTSIZE | XFS_AT_PROJID;
> - vattr->va_xflags = fa.fsx_xflags;
> - vattr->va_extsize = fa.fsx_extsize;
> - vattr->va_projid = fa.fsx_projid;
> -
> - error = xfs_setattr(ip, vattr, attr_flags, NULL);
> - if (likely(!error))
*HERE*
> - vn_revalidate(XFS_ITOV(ip)); /* update flags */
> - error = -error;
> - break;
> - }
> -
> - case XFS_IOC_GETXFLAGS: {
> - flags = xfs_di2lxflags(ip->i_d.di_flags);
> - if (copy_to_user(arg, &flags, sizeof(flags)))
> - error = -EFAULT;
> - break;
> - }
> -
> - case XFS_IOC_SETXFLAGS: {
> - if (copy_from_user(&flags, arg, sizeof(flags))) {
> - error = -EFAULT;
> - break;
> - }
> -
> - if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
> - FS_NOATIME_FL | FS_NODUMP_FL | \
> - FS_SYNC_FL)) {
> - error = -EOPNOTSUPP;
> - break;
> - }
> -
> - attr_flags = 0;
> - if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> - attr_flags |= ATTR_NONBLOCK;
> -
> - vattr->va_mask = XFS_AT_XFLAGS;
> - vattr->va_xflags = xfs_merge_ioc_xflags(flags,
> - xfs_ip2xflags(ip));
> -
> - error = xfs_setattr(ip, vattr, attr_flags, NULL);
> - if (likely(!error))
*HERE*
> - vn_revalidate(XFS_ITOV(ip)); /* update flags */
> - error = -error;
> - break;
> - }
> + attr_flags = 0;
> + if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> + attr_flags |= ATTR_NONBLOCK;
> +
> + vattr->va_mask = XFS_AT_XFLAGS | XFS_AT_EXTSIZE | XFS_AT_PROJID;
> + vattr->va_xflags = fa.fsx_xflags;
> + vattr->va_extsize = fa.fsx_extsize;
> + vattr->va_projid = fa.fsx_projid;
> +
> + error = -xfs_setattr(ip, vattr, attr_flags, NULL);
> + if (!error)
> + vn_revalidate(XFS_ITOV(ip)); /* update flags */
> + kfree(vattr);
> + return 0;
> +}
>
> - default:
> - error = -ENOTTY;
> - break;
> - }
> +STATIC int
> +xfs_ioc_getxflags(
> + xfs_inode_t *ip,
> + void __user *arg)
> +{
> + unsigned int flags;
> +
> + flags = xfs_di2lxflags(ip->i_d.di_flags);
> + if (copy_to_user(arg, &flags, sizeof(flags)))
> + return -EFAULT;
> + return 0;
> +}
>
> +STATIC int
> +xfs_ioc_setxflags(
> + xfs_inode_t *ip,
> + struct file *filp,
> + void __user *arg)
> +{
> + struct bhv_vattr *vattr;
> + unsigned int flags;
> + int attr_flags;
> + int error;
> +
> + if (copy_from_user(&flags, arg, sizeof(flags)))
> + return -EFAULT;
> +
> + if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \
> + FS_NOATIME_FL | FS_NODUMP_FL | \
> + FS_SYNC_FL))
> + return -EOPNOTSUPP;
> +
> + vattr = kmalloc(sizeof(*vattr), GFP_KERNEL);
> + if (unlikely(!vattr))
*HERE*
> + return -ENOMEM;
> +
> + attr_flags = 0;
> + if (filp->f_flags & (O_NDELAY|O_NONBLOCK))
> + attr_flags |= ATTR_NONBLOCK;
> +
> + vattr->va_mask = XFS_AT_XFLAGS;
> + vattr->va_xflags = xfs_merge_ioc_xflags(flags, xfs_ip2xflags(ip));
> +
> + error = -xfs_setattr(ip, vattr, attr_flags, NULL);
> + if (likely(!error))
*HERE*
> + vn_revalidate(XFS_ITOV(ip)); /* update flags */
> kfree(vattr);
> return error;
> }
> @@ -1090,10 +1090,12 @@ xfs_ioctl(
> return xfs_ioc_fsgetxattr(ip, 0, arg);
> case XFS_IOC_FSGETXATTRA:
> return xfs_ioc_fsgetxattr(ip, 1, arg);
> + case XFS_IOC_FSSETXATTR:
> + return xfs_ioc_fssetxattr(ip, filp, arg);
> case XFS_IOC_GETXFLAGS:
> + return xfs_ioc_getxflags(ip, arg);
> case XFS_IOC_SETXFLAGS:
> - case XFS_IOC_FSSETXATTR:
> - return xfs_ioc_xattr(ip, filp, cmd, arg);
> + return xfs_ioc_setxflags(ip, filp, arg);
>
> case XFS_IOC_FSSETDM: {
> struct fsdmidata dmi;
--
Niv Sardi
|