Hi Dave,
On 06/24/2013 07:13 AM, Dave Chinner wrote:
> On Fri, Jun 21, 2013 at 09:27:58PM +0800, Jeff Liu wrote:
>> From: Jie Liu <jeff.liu@xxxxxxxxxx>
>>
>> Consolidate lseek(2) SEEK_DATA/SEEK_HOLE according to the
>> implementation of VFS lseek_execute():
>> - if end up with a negative offset, return EINVAL if file
>> is not huge.
>> - if end up with an offset larger than s_maxbytes, return
>> EINVAL as well.
>> - reset file version to 0 if end up with an offset that is
>> not equal to the current file offset.
>>
>> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>>
>> ---
>> fs/xfs/xfs_file.c | 26 ++++++++++++++++++++++++--
>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
>> index a5f2042..dc42751 100644
>> --- a/fs/xfs/xfs_file.c
>> +++ b/fs/xfs/xfs_file.c
>> @@ -1270,8 +1270,19 @@ xfs_seek_data(
>> }
>>
>> out:
>> - if (offset != file->f_pos)
>> + if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
>> + error = EINVAL;
>> + goto out_unlock;
>> + }
>> + if (offset > inode->i_sb->s_maxbytes) {
>> + error = EINVAL;
>> + goto out_unlock;
>> + }
>> +
>> + if (offset != file->f_pos) {
>> file->f_pos = offset;
>> + file->f_version = 0;
>> + }
>
> Hi Jeff, why are you copy-n-pasting this code from lseek_execute()
> rather than making lseek_execute() an exported function and calling
> that directly?
I found other file systems are implemented in this way. But I should
consider how to make it better rather than following others in this
situation.
>
>>
>> out_unlock:
>> xfs_iunlock_map_shared(ip, lock);
>> @@ -1372,6 +1383,15 @@ xfs_seek_hole(
>> }
>>
>> out:
>> + if (offset < 0 && !(file->f_mode & FMODE_UNSIGNED_OFFSET)) {
>> + error = EINVAL;
>> + goto out_unlock;
>> + }
>> + if (offset > inode->i_sb->s_maxbytes) {
>> + error = EINVAL;
>> + goto out_unlock;
>> + }
>
> These checks belong after we truncated offset to isize, don't they?
They do.
> And that would make both of these functions simply require a call to
> lseek_execute(), yes?
Yep, I'll post a patch set to export this call and propagate it to other
file systems as well.
Thanks,
-Jeff
|