* [PATCH] relay: When unsigned ret cannot store a negative value @ 2008-11-29 15:38 roel kluin 2008-12-02 22:10 ` Andrew Morton 0 siblings, 1 reply; 5+ messages in thread From: roel kluin @ 2008-11-29 15:38 UTC (permalink / raw) To: linux-kernel When unsigned ret cannot store a negative value Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- UNTESTED! is this the way to go? diff --git a/kernel/relay.c b/kernel/relay.c index 32b0bef..c9d62e5 100644 --- a/kernel/relay.c +++ b/kernel/relay.c @@ -1220,7 +1220,8 @@ static int subbuf_splice_actor(struct file *in, unsigned int flags, int *nonpad_ret) { - unsigned int pidx, poff, total_len, subbuf_pages, nr_pages, ret; + unsigned int pidx, poff, total_len, subbuf_pages, nr_pages; + ssize_t ret; struct rchan_buf *rbuf = in->private_data; unsigned int subbuf_size = rbuf->chan->subbuf_size; uint64_t pos = (uint64_t) *ppos; @@ -1289,7 +1290,8 @@ static int subbuf_splice_actor(struct file *in, if (!spd.nr_pages) return 0; - ret = *nonpad_ret = splice_to_pipe(pipe, &spd); + ret = splice_to_pipe(pipe, &spd); + *nonpad_ret = ret; if (ret < 0 || ret < total_len) return ret; ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] relay: When unsigned ret cannot store a negative value 2008-11-29 15:38 [PATCH] relay: When unsigned ret cannot store a negative value roel kluin @ 2008-12-02 22:10 ` Andrew Morton 2008-12-03 4:29 ` Tom Zanussi 0 siblings, 1 reply; 5+ messages in thread From: Andrew Morton @ 2008-12-02 22:10 UTC (permalink / raw) To: roel kluin; +Cc: linux-kernel, Tom Zanussi, Tom Zanussi, Jens Axboe On Sat, 29 Nov 2008 10:38:06 -0500 roel kluin <roel.kluin@gmail.com> wrote: > When unsigned ret cannot store a negative value > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > --- > UNTESTED! is this the way to go? > > diff --git a/kernel/relay.c b/kernel/relay.c > index 32b0bef..c9d62e5 100644 > --- a/kernel/relay.c > +++ b/kernel/relay.c > @@ -1220,7 +1220,8 @@ static int subbuf_splice_actor(struct file *in, > unsigned int flags, > int *nonpad_ret) > { > - unsigned int pidx, poff, total_len, subbuf_pages, nr_pages, ret; > + unsigned int pidx, poff, total_len, subbuf_pages, nr_pages; > + ssize_t ret; > struct rchan_buf *rbuf = in->private_data; > unsigned int subbuf_size = rbuf->chan->subbuf_size; > uint64_t pos = (uint64_t) *ppos; > @@ -1289,7 +1290,8 @@ static int subbuf_splice_actor(struct file *in, > if (!spd.nr_pages) > return 0; > > - ret = *nonpad_ret = splice_to_pipe(pipe, &spd); > + ret = splice_to_pipe(pipe, &spd); > + *nonpad_ret = ret; > if (ret < 0 || ret < total_len) > return ret; > Yeah, this code needs help. subbuf_splice_actor() returns `int', but carefully calculates and returns an unsigned type. I suspect that quite a bit of code in there (including relay_file_splice_read()) should be gone through and have its choice of types reviewed and fixed. Probably by converting things to ssize_t. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] relay: When unsigned ret cannot store a negative value 2008-12-02 22:10 ` Andrew Morton @ 2008-12-03 4:29 ` Tom Zanussi 2008-12-03 4:37 ` Andrew Morton 0 siblings, 1 reply; 5+ messages in thread From: Tom Zanussi @ 2008-12-03 4:29 UTC (permalink / raw) To: Andrew Morton; +Cc: roel kluin, linux-kernel, Tom Zanussi, Jens Axboe On Tue, 2008-12-02 at 14:10 -0800, Andrew Morton wrote: > On Sat, 29 Nov 2008 10:38:06 -0500 > roel kluin <roel.kluin@gmail.com> wrote: > > > When unsigned ret cannot store a negative value > > > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > > --- > > UNTESTED! is this the way to go? > > > > diff --git a/kernel/relay.c b/kernel/relay.c > > index 32b0bef..c9d62e5 100644 > > --- a/kernel/relay.c > > +++ b/kernel/relay.c > > @@ -1220,7 +1220,8 @@ static int subbuf_splice_actor(struct file *in, > > unsigned int flags, > > int *nonpad_ret) > > { > > - unsigned int pidx, poff, total_len, subbuf_pages, nr_pages, ret; > > + unsigned int pidx, poff, total_len, subbuf_pages, nr_pages; > > + ssize_t ret; > > struct rchan_buf *rbuf = in->private_data; > > unsigned int subbuf_size = rbuf->chan->subbuf_size; > > uint64_t pos = (uint64_t) *ppos; > > @@ -1289,7 +1290,8 @@ static int subbuf_splice_actor(struct file *in, > > if (!spd.nr_pages) > > return 0; > > > > - ret = *nonpad_ret = splice_to_pipe(pipe, &spd); > > + ret = splice_to_pipe(pipe, &spd); > > + *nonpad_ret = ret; > > if (ret < 0 || ret < total_len) > > return ret; > > > > Yeah, this code needs help. subbuf_splice_actor() returns `int', but > carefully calculates and returns an unsigned type. > > I suspect that quite a bit of code in there (including > relay_file_splice_read()) should be gone through and have its choice of > types reviewed and fixed. Probably by converting things to ssize_t. > > And maybe generic_file_splice_read() too, from whence it came. Speaking of which, is there some reason why this patch: http://lkml.indiana.edu/hypermail/linux/kernel/0810.3/0094.html never got picked up? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] relay: When unsigned ret cannot store a negative value 2008-12-03 4:29 ` Tom Zanussi @ 2008-12-03 4:37 ` Andrew Morton 2008-12-03 5:19 ` Tom Zanussi 0 siblings, 1 reply; 5+ messages in thread From: Andrew Morton @ 2008-12-03 4:37 UTC (permalink / raw) To: Tom Zanussi; +Cc: roel kluin, linux-kernel, Tom Zanussi, Jens Axboe On Tue, 02 Dec 2008 22:29:51 -0600 Tom Zanussi <zanussi@comcast.net> wrote: > > On Tue, 2008-12-02 at 14:10 -0800, Andrew Morton wrote: > > On Sat, 29 Nov 2008 10:38:06 -0500 > > roel kluin <roel.kluin@gmail.com> wrote: > > > > > When unsigned ret cannot store a negative value > > > > > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > > > --- > > > UNTESTED! is this the way to go? > > > > > > diff --git a/kernel/relay.c b/kernel/relay.c > > > index 32b0bef..c9d62e5 100644 > > > --- a/kernel/relay.c > > > +++ b/kernel/relay.c > > > @@ -1220,7 +1220,8 @@ static int subbuf_splice_actor(struct file *in, > > > unsigned int flags, > > > int *nonpad_ret) > > > { > > > - unsigned int pidx, poff, total_len, subbuf_pages, nr_pages, ret; > > > + unsigned int pidx, poff, total_len, subbuf_pages, nr_pages; > > > + ssize_t ret; > > > struct rchan_buf *rbuf = in->private_data; > > > unsigned int subbuf_size = rbuf->chan->subbuf_size; > > > uint64_t pos = (uint64_t) *ppos; > > > @@ -1289,7 +1290,8 @@ static int subbuf_splice_actor(struct file *in, > > > if (!spd.nr_pages) > > > return 0; > > > > > > - ret = *nonpad_ret = splice_to_pipe(pipe, &spd); > > > + ret = splice_to_pipe(pipe, &spd); > > > + *nonpad_ret = ret; > > > if (ret < 0 || ret < total_len) > > > return ret; > > > > > > > Yeah, this code needs help. subbuf_splice_actor() returns `int', but > > carefully calculates and returns an unsigned type. > > > > I suspect that quite a bit of code in there (including > > relay_file_splice_read()) should be gone through and have its choice of > > types reviewed and fixed. Probably by converting things to ssize_t. > > > > > > And maybe generic_file_splice_read() too, from whence it came. sure. > Speaking > of which, is there some reason why this patch: > > http://lkml.indiana.edu/hypermail/linux/kernel/0810.3/0094.html > > never got picked up? > Incompetence I guess. Damn man, that's your third email address :( ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] relay: When unsigned ret cannot store a negative value 2008-12-03 4:37 ` Andrew Morton @ 2008-12-03 5:19 ` Tom Zanussi 0 siblings, 0 replies; 5+ messages in thread From: Tom Zanussi @ 2008-12-03 5:19 UTC (permalink / raw) To: Andrew Morton; +Cc: roel kluin, linux-kernel, Jens Axboe On Tue, 2008-12-02 at 20:37 -0800, Andrew Morton wrote: > On Tue, 02 Dec 2008 22:29:51 -0600 Tom Zanussi <zanussi@comcast.net> wrote: > > > > > On Tue, 2008-12-02 at 14:10 -0800, Andrew Morton wrote: > > > On Sat, 29 Nov 2008 10:38:06 -0500 > > > roel kluin <roel.kluin@gmail.com> wrote: > > > > > > > When unsigned ret cannot store a negative value > > > > > > > > Signed-off-by: Roel Kluin <roel.kluin@gmail.com> > > > > --- > > > > UNTESTED! is this the way to go? > > > > > > > > diff --git a/kernel/relay.c b/kernel/relay.c > > > > index 32b0bef..c9d62e5 100644 > > > > --- a/kernel/relay.c > > > > +++ b/kernel/relay.c > > > > @@ -1220,7 +1220,8 @@ static int subbuf_splice_actor(struct file *in, > > > > unsigned int flags, > > > > int *nonpad_ret) > > > > { > > > > - unsigned int pidx, poff, total_len, subbuf_pages, nr_pages, ret; > > > > + unsigned int pidx, poff, total_len, subbuf_pages, nr_pages; > > > > + ssize_t ret; > > > > struct rchan_buf *rbuf = in->private_data; > > > > unsigned int subbuf_size = rbuf->chan->subbuf_size; > > > > uint64_t pos = (uint64_t) *ppos; > > > > @@ -1289,7 +1290,8 @@ static int subbuf_splice_actor(struct file *in, > > > > if (!spd.nr_pages) > > > > return 0; > > > > > > > > - ret = *nonpad_ret = splice_to_pipe(pipe, &spd); > > > > + ret = splice_to_pipe(pipe, &spd); > > > > + *nonpad_ret = ret; > > > > if (ret < 0 || ret < total_len) > > > > return ret; > > > > > > > > > > Yeah, this code needs help. subbuf_splice_actor() returns `int', but > > > carefully calculates and returns an unsigned type. > > > > > > I suspect that quite a bit of code in there (including > > > relay_file_splice_read()) should be gone through and have its choice of > > > types reviewed and fixed. Probably by converting things to ssize_t. > > > > > > > > > > And maybe generic_file_splice_read() too, from whence it came. > > sure. > > > Speaking > > of which, is there some reason why this patch: > > > > http://lkml.indiana.edu/hypermail/linux/kernel/0810.3/0094.html > > > > never got picked up? > > > > Incompetence I guess. No problem, thanks for finally picking it up. :-) > > Damn man, that's your third email address :( Yeah, the purpose of the gmail address is to avoid that but I forget to use it - will try to be less careless in the future. ;-) ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-12-03 5:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-11-29 15:38 [PATCH] relay: When unsigned ret cannot store a negative value roel kluin 2008-12-02 22:10 ` Andrew Morton 2008-12-03 4:29 ` Tom Zanussi 2008-12-03 4:37 ` Andrew Morton 2008-12-03 5:19 ` Tom Zanussi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox