Re: [PATCH] vfs: fix vmplice_to_user()

[Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>]

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);