lkml.org 
[lkml]   [2009]   [Dec]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] relay: Fix signedness issues in relay_page_release() and relay_file_splice_read()
In subbuf_splice_actor() `ret' is unsigned, so the `ret < 0'
test does not work and splice_to_pipe() errors are not acted upon.
It appears the cause is confusion between the signedness of the
return values `ret' and `nonpad_ret'.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Found using coccinelle: http://coccinelle.lip6.fr/

I tried to solve this in the patch below, A remaining problem may be
that `spliced' wraps, any comments?

The rationale for the patch below:

padding (size_t) is only added to `ret' and not to `*nonpad_ret', so
I think that ret should be unsigned, or even size_t, whereas `*nonpad_ret'
should be int and carry the error.

the caller relay_file_splice_read() should therefore check the error in
`nonpad_ret', which should be int, rather than in `ret' - unsigned.

checkpatch likes it and it appears to build, but maybe there are
comments?

diff --git a/kernel/relay.c b/kernel/relay.c
index 760c262..632fca2 100644
--- a/kernel/relay.c
+++ b/kernel/relay.c
@@ -1215,7 +1215,7 @@ static void relay_page_release(struct splice_pipe_desc *spd, unsigned int i)
/*
* subbuf_splice_actor - splice up to one subbuf's worth of data
*/
-static int subbuf_splice_actor(struct file *in,
+static unsigned int subbuf_splice_actor(struct file *in,
loff_t *ppos,
struct pipe_inode_info *pipe,
size_t len,
@@ -1292,7 +1292,7 @@ static int subbuf_splice_actor(struct file *in,
return 0;

ret = *nonpad_ret = splice_to_pipe(pipe, &spd);
- if (ret < 0 || ret < total_len)
+ if (*nonpad_ret < 0 || ret < total_len)
return ret;

if (read_start + ret == nonpad_end)
@@ -1308,7 +1308,7 @@ static ssize_t relay_file_splice_read(struct file *in,
unsigned int flags)
{
ssize_t spliced;
- int ret;
+ unsigned ret;
int nonpad_ret = 0;

ret = 0;
@@ -1316,11 +1316,11 @@ static ssize_t relay_file_splice_read(struct file *in,

while (len && !spliced) {
ret = subbuf_splice_actor(in, ppos, pipe, len, flags, &nonpad_ret);
- if (ret < 0)
+ if (nonpad_ret < 0)
break;
else if (!ret) {
if (flags & SPLICE_F_NONBLOCK)
- ret = -EAGAIN;
+ nonpad_ret = -EAGAIN;
break;
}

@@ -1336,7 +1336,7 @@ static ssize_t relay_file_splice_read(struct file *in,
if (spliced)
return spliced;

- return ret;
+ return nonpad_ret;
}

const struct file_operations relay_file_operations = {

\
 
 \ /
  Last update: 2009-12-15 22:29    [W:0.029 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site