> On Oct 24, 2015, at 10:52 AM, Eric Biggers <ebigge...@gmail.com> wrote: > > A few comments: > >> if (!(file_in->f_mode & FMODE_READ) || >> !(file_out->f_mode & FMODE_WRITE) || >> (file_out->f_flags & O_APPEND) || >> !file_out->f_op) >> return -EBADF; > > Isn't 'f_op' always non-NULL? > > If the destination file cannot be append-only, shouldn't this be documented?
Actually, wouldn't O_APPEND only be a problem if the target file wasn't
being appended to? In other words, if the target i_size == start offset
then it should be possible to use the copy syscall on an O_APPEND file.
Cheers, Andreas
>> if (inode_in->i_sb != inode_out->i_sb ||
>> file_in->f_path.mnt != file_out->f_path.mnt)
>> return -EXDEV;
>
> Doesn't the same mount already imply the same superblock?
>
>> /*
>> * copy_file_range() differs from regular file read and write in that it
>> * specifically allows return partial success. When it does so is up to
>> * the copy_file_range method.
>> */
>
> What does this mean? I thought that read() and write() can also return
> partial
> success.
>
>> f_out = fdget(fd_out);
>> if (!f_in.file || !f_out.file) {
>> ret = -EBADF;
>> goto out;
>> }
>
> This looked wrong at first because it may call fdput() on a 'struct fd' that
> was
> not successfully acquired, but it looks like it works currently because of how
> the FDPUT_FPUT flag is used. It may be a good idea to write it the "obvious"
> way, though (use separate labels depending on which fdput()s need to happen).
>
>
> Other questions:
>
> Should FMODE_PREAD or FMODE_PWRITE access be checked if the user specifies
> their
> own 'off_in' or 'off_out', respectively?
>
> What is supposed to happen if the user passes provides a file descriptor to a
> non-regular file, such as a block device or char device?
>
> If the 'in' file has fewer than 'len' bytes remaining until EOF, what is the
> expected behavior? It looks like the btrfs implementation has different
> behavior from the pagecache implementation.
>
> It appears the btrfs implementation has alignment restrictions --- where is
> this
> documented and how will users know what alignment to use?
>
> Are copies within the same file permitted and can the ranges overlap? The man
> page doesn't say.
>
> It looks like the initial patch defines __NR_copy_file_range for the ARM
> architecture but doesn't actually hook that system call up for ARM; why is
> that?
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers, Andreas
signature.asc
Description: Message signed with OpenPGP using GPGMail
