| Message ID | d99f78a7-31c4-582e-17f5-93e1f0d0e4c2@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fuse: BUG_ON's correction in fuse_dev_splice_write() | expand |
On Tue, Jul 23, 2019 at 8:33 AM Vasily Averin <vvs@virtuozzo.com> wrote: > > commit 963545357202 ("fuse: reduce allocation size for splice_write") > changed size of bufs array, so first BUG_ON should be corrected too. > Second BUG_ON become useless, first one also includes the second check: > any unsigned nbuf value cannot be less than 0. This patch seems broken: it assumes that pipe->nrbufs doesn't change. Have you actually tested it? Thanks, Miklos
On 8/1/19 2:01 PM, Miklos Szeredi wrote: > On Tue, Jul 23, 2019 at 8:33 AM Vasily Averin <vvs@virtuozzo.com> wrote: >> >> commit 963545357202 ("fuse: reduce allocation size for splice_write") >> changed size of bufs array, so first BUG_ON should be corrected too. >> Second BUG_ON become useless, first one also includes the second check: >> any unsigned nbuf value cannot be less than 0. > > This patch seems broken: it assumes that pipe->nrbufs doesn't change. > Have you actually tested it? You're right, I've missed it. I've prepared second patch version which fixes first BUG_ON only. checkpatch.pl also advises to replace BUG_ONs to WARN_ONs and 'unsigned' to 'unsigned int' however I'm don't understand what it's better here: - keep all as is, - or merge all changes together, - or do it in separate patches, - or do something else? I believe it makes sense to remove BUG_ONs in separate patch, or may be merge it with current one, but I do not like an idea to fight against bare 'unsigned' in fuse. Could you please comment it? Thank you, Vasily Averin
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c index ea8237513dfa..311c7911271c 100644 --- a/fs/fuse/dev.c +++ b/fs/fuse/dev.c @@ -2064,8 +2064,7 @@ static ssize_t fuse_dev_splice_write(struct pipe_inode_info *pipe, struct pipe_buffer *ibuf; struct pipe_buffer *obuf; - BUG_ON(nbuf >= pipe->buffers); - BUG_ON(!pipe->nrbufs); + BUG_ON(nbuf >= pipe->nrbufs); ibuf = &pipe->bufs[pipe->curbuf]; obuf = &bufs[nbuf];
commit 963545357202 ("fuse: reduce allocation size for splice_write") changed size of bufs array, so first BUG_ON should be corrected too. Second BUG_ON become useless, first one also includes the second check: any unsigned nbuf value cannot be less than 0. Fixes: 963545357202 ("fuse: reduce allocation size for splice_write") Signed-off-by: Vasily Averin <vvs@virtuozzo.com> --- fs/fuse/dev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)