Re: [PATCH] vfs: fix vmplice_to_user()

[Linus Torvalds]

On Tue, May 27, 2014 at 7:41 AM, Miklos Szeredi <mik...@szeredi.hu> 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);