xfs
[Top] [All Lists]

Re: [patch 03/11] Add compat handlers for data & rt growfs ioctls

To: sandeen@xxxxxxxxxxx
Subject: Re: [patch 03/11] Add compat handlers for data & rt growfs ioctls
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 19 Nov 2008 10:07:16 -0500
Cc: xfs@xxxxxxxxxxx, hch@xxxxxxxxxxxxx
In-reply-to: <20081119044908.507467044@xxxxxxxxxxx>
References: <20081119044401.573365619@xxxxxxxxxxx> <20081119044908.507467044@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Tue, Nov 18, 2008 at 10:44:04PM -0600, sandeen@xxxxxxxxxxx wrote:
> The args for XFS_IOC_FSGROWFSDATA and XFS_IOC_FSGROWFSRTA
> have padding on the end on intel, so add arg copyin functions,
> and then just call the native ioctl.
> 
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxxx>
> --
> 
> Index: linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl32.c
> ===================================================================
> --- linux-2.6-xfs.orig/fs/xfs/linux-2.6/xfs_ioctl32.c
> +++ linux-2.6-xfs/fs/xfs/linux-2.6/xfs_ioctl32.c
> @@ -87,6 +87,28 @@ xfs_ioc_fsgeometry_v1_compat(
>       return 0;
>  }
>  
> +STATIC unsigned long
> +xfs_ioctl32_growfs_data_copyin(unsigned long arg)
> +{
> +     compat_xfs_growfs_data_t __user *p32 = (void __user *)arg;
> +     xfs_growfs_data_t __user *p = compat_alloc_user_space(sizeof(*p));
> +
> +     if (copy_in_user(p, p32, sizeof(*p32)))
> +             return -XFS_ERROR(EFAULT);
> +     return (unsigned long)p;
> +}

As mentioned in reply to the first patch I think this would be better
as:

STATIC unsigned long
xfs_ioctl32_growfs_data(
        struct xfs_mount                *mp,
        void __user                     *argp)
{
        compat_xfs_growfs_data_t        in32;
        xfs_growfs_data_t               in;

        if (copy_from_user(&in, arg, sizeof(in32)))
                return -XFS_ERROR(EFAULT);
        in.newblocks = in32.newblocks;
        in.imaxpct = in32.imaxpct;
        return -xfs_growfs_data(mp, &in);
}

and same for the rt version.  Also all these alignment only adjustments
really needs comments explaining what we're doing here as it's not
obvious.

<Prev in Thread] Current Thread [Next in Thread>