xfs
[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [patch 01/11] Move compat ioctl structs & numbers into xfs_ioctl32.h
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Wed, 19 Nov 2008 09:24:28 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <20081119145941.GA13050@xxxxxxxxxxxxx>
References: <20081119044401.573365619@xxxxxxxxxxx> <20081119044907.776640320@xxxxxxxxxxx> <20081119145941.GA13050@xxxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.17 (Macintosh/20080914)
Christoph Hellwig wrote:
> 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?

eh, not really.  I can change it back.

> 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.

ok, sounds good.

> 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.

Ok, either way is fine by me, was just following what was there already.

>> -#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.
> 

ah, good point.

-Eric

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