Re: [PATCH] vfs: fix vmplice_to_user()

From: Linus Torvalds
Date: Tue May 27 2014 - 13:05:08 EST


On Tue, May 27, 2014 at 7:41 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> This patch fixes two bugs:

Mind if I change it to avoid the "goto"? I don't think goto is evil,
but in this case it doesn't seem to buy anything, except to perhaps
make the patch a bit smaller at the expense of making the result a bit
harder to read.

Replacing the

if (ret <= 0)
goto out;

with

if (ret > 0) {
...
}

also allows moving the various variable declarations closer to their use.

So something like the attached..

Linus
fs/splice.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/splice.c b/fs/splice.c
index 9bc07d2b53cf..6b1115005150 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -1532,12 +1532,9 @@ static long vmsplice_to_user(struct file *file, const struct iovec __user *uiov,
unsigned long nr_segs, unsigned int flags)
{
struct pipe_inode_info *pipe;
- struct splice_desc sd;
- long ret;
struct iovec iovstack[UIO_FASTIOV];
struct iovec *iov = iovstack;
- struct iov_iter iter;
- ssize_t count = 0;
+ long ret;

pipe = get_pipe_info(file);
if (!pipe)
@@ -1545,20 +1542,23 @@ static long vmsplice_to_user(struct file *file, const struct iovec __user *uiov,

ret = rw_copy_check_uvector(READ, uiov, nr_segs,
ARRAY_SIZE(iovstack), iovstack, &iov);
- if (ret <= 0)
- return ret;
+ if (ret > 0) {
+ struct splice_desc sd;
+ struct iov_iter iter;
+ ssize_t count = ret;

- iov_iter_init(&iter, iov, nr_segs, count, 0);
+ iov_iter_init(&iter, iov, nr_segs, count, 0);

- sd.len = 0;
- sd.total_len = count;
- sd.flags = flags;
- sd.u.data = &iter;
- sd.pos = 0;
+ sd.len = 0;
+ sd.total_len = count;
+ sd.flags = flags;
+ sd.u.data = &iter;
+ sd.pos = 0;

- pipe_lock(pipe);
- ret = __splice_from_pipe(pipe, &sd, pipe_to_user);
- pipe_unlock(pipe);
+ pipe_lock(pipe);
+ ret = __splice_from_pipe(pipe, &sd, pipe_to_user);
+ pipe_unlock(pipe);
+ }

if (iov != iovstack)
kfree(iov);