xfs
[Top] [All Lists]

Re: [patch 01/11] Move compat ioctl structs & numbers into xfs_ioctl32.h

To: sandeen@xxxxxxxxxxx
Subject: Re: [patch 01/11] Move compat ioctl structs & numbers into xfs_ioctl32.h
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 19 Nov 2008 09:59:41 -0500
Cc: xfs@xxxxxxxxxxx, hch@xxxxxxxxxxxxx
In-reply-to: <20081119044907.776640320@xxxxxxxxxxx>
References: <20081119044401.573365619@xxxxxxxxxxx> <20081119044907.776640320@xxxxxxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Tue, Nov 18, 2008 at 10:44:02PM -0600, sandeen@xxxxxxxxxxx wrote:
> +#ifdef BROKEN_X86_ALIGNMENT
>  STATIC unsigned long
> -xfs_ioctl32_flock(
> -     unsigned long           arg)
> +xfs_ioctl32_flock_copyin(unsigned long arg)

Any reason this deflect from the standard xfs formatting?

Also when you touch these helpers it would be a lot nice to already
pass a void __user * pointer to them with just a single case in the main
handler ala

        void__user *    argp = (void __user *)arg;

And then use that directly in the places where it works, e.g.
copy_{from,to,in}_user.

I must also say that I don't really like these _copying helpers at all,
just adding an explicit call to the underlying ioctl from them seems
much clener than dispatching control back to the main routine with a
changed argument and ioc number.

> -#define XFS_IOC32_GETXFLAGS  FS_IOC32_GETFLAGS
> -#define XFS_IOC32_SETXFLAGS  FS_IOC32_SETFLAGS
> -#define XFS_IOC32_GETVERSION FS_IOC32_GETVERSION
> +#define XFS_IOC_GETXFLAGS_32 FS_IOC32_GETFLAGS
> +#define XFS_IOC_SETXFLAGS_32 FS_IOC32_SETFLAGS
> +#define XFS_IOC_GETVERSION_32        FS_IOC32_GETVERSION

I think these should go into xfs_ioctl32.h, too.

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