* [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' @ 2017-12-07 23:03 Vasyl Gomonovych 2017-12-07 23:20 ` Matthew Wilcox 2017-12-07 23:23 ` Al Viro 0 siblings, 2 replies; 5+ messages in thread From: Vasyl Gomonovych @ 2017-12-07 23:03 UTC (permalink / raw) To: viro, linux-fsdevel, gomonovych; +Cc: linux-kernel p could be NULL and passing into PTR_ERR Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com> --- fs/seq_file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/seq_file.c b/fs/seq_file.c index 4be761c..8b700b9 100644 --- a/fs/seq_file.c +++ b/fs/seq_file.c @@ -262,8 +262,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) size_t offs = m->count; loff_t next = pos; p = m->op->next(m, p, &next); - if (!p || IS_ERR(p)) { - err = PTR_ERR(p); + if (IS_ERR(p)) { + err = (!p ? -EFAULT : PTR_ERR(p)); break; } err = m->op->show(m, p); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' 2017-12-07 23:03 [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' Vasyl Gomonovych @ 2017-12-07 23:20 ` Matthew Wilcox 2017-12-07 23:23 ` Al Viro 1 sibling, 0 replies; 5+ messages in thread From: Matthew Wilcox @ 2017-12-07 23:20 UTC (permalink / raw) To: Vasyl Gomonovych; +Cc: viro, linux-fsdevel, linux-kernel On Fri, Dec 08, 2017 at 12:03:07AM +0100, Vasyl Gomonovych wrote: > p could be NULL and passing into PTR_ERR What makes you think this is correct? To quote the documentation: The next function to implement is called, amazingly, next(); its job is to move the iterator forward to the next position in the sequence. The example module can simply increment the position by one; more useful modules will do what is needed to step through some data structure. The next() function returns a new iterator, or NULL if the sequence is complete. Here's the example version: So if it returns NULL, we want to set err to 0 and break. Which is, um, exactly what the code does. Did you test this at all? > Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com> > --- > fs/seq_file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/seq_file.c b/fs/seq_file.c > index 4be761c..8b700b9 100644 > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -262,8 +262,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) > size_t offs = m->count; > loff_t next = pos; > p = m->op->next(m, p, &next); > - if (!p || IS_ERR(p)) { > - err = PTR_ERR(p); > + if (IS_ERR(p)) { > + err = (!p ? -EFAULT : PTR_ERR(p)); > break; > } > err = m->op->show(m, p); > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' 2017-12-07 23:03 [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' Vasyl Gomonovych 2017-12-07 23:20 ` Matthew Wilcox @ 2017-12-07 23:23 ` Al Viro 2017-12-07 23:26 ` Al Viro 1 sibling, 1 reply; 5+ messages in thread From: Al Viro @ 2017-12-07 23:23 UTC (permalink / raw) To: Vasyl Gomonovych; +Cc: linux-fsdevel, linux-kernel On Fri, Dec 08, 2017 at 12:03:07AM +0100, Vasyl Gomonovych wrote: > p could be NULL and passing into PTR_ERR > > Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com> > --- > fs/seq_file.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/seq_file.c b/fs/seq_file.c > index 4be761c..8b700b9 100644 > --- a/fs/seq_file.c > +++ b/fs/seq_file.c > @@ -262,8 +262,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) > size_t offs = m->count; > loff_t next = pos; > p = m->op->next(m, p, &next); > - if (!p || IS_ERR(p)) { > - err = PTR_ERR(p); > + if (IS_ERR(p)) { > + err = (!p ? -EFAULT : PTR_ERR(p)); What does it fix, if I might ask? And while we are at it, would you mind explaining the reasoning behind that change? Or, say, testing done to it... ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' 2017-12-07 23:23 ` Al Viro @ 2017-12-07 23:26 ` Al Viro 2017-12-08 7:54 ` Gomonovych, Vasyl 0 siblings, 1 reply; 5+ messages in thread From: Al Viro @ 2017-12-07 23:26 UTC (permalink / raw) To: Vasyl Gomonovych; +Cc: linux-fsdevel, linux-kernel On Thu, Dec 07, 2017 at 11:23:26PM +0000, Al Viro wrote: > On Fri, Dec 08, 2017 at 12:03:07AM +0100, Vasyl Gomonovych wrote: > > p could be NULL and passing into PTR_ERR > > > > Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com> > > --- > > fs/seq_file.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/seq_file.c b/fs/seq_file.c > > index 4be761c..8b700b9 100644 > > --- a/fs/seq_file.c > > +++ b/fs/seq_file.c > > @@ -262,8 +262,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) > > size_t offs = m->count; > > loff_t next = pos; > > p = m->op->next(m, p, &next); > > - if (!p || IS_ERR(p)) { > > - err = PTR_ERR(p); > > + if (IS_ERR(p)) { > > + err = (!p ? -EFAULT : PTR_ERR(p)); > > What does it fix, if I might ask? And while we are at it, would > you mind explaining the reasoning behind that change? Or, say, > testing done to it... While we are at it, where has that -EFAULT come from? And how would it be ever reached, seeing that IS_ERR(NULL) is false? ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' 2017-12-07 23:26 ` Al Viro @ 2017-12-08 7:54 ` Gomonovych, Vasyl 0 siblings, 0 replies; 5+ messages in thread From: Gomonovych, Vasyl @ 2017-12-08 7:54 UTC (permalink / raw) To: Al Viro, willy; +Cc: linux-fsdevel, linux-kernel Hi, Guys sorry for this idiotic piece of code. Yesterday after doc seq_file.txt read I did not catch real way of work there. And made this shit. Sorry. Regards Vasyl On Fri, Dec 8, 2017 at 12:26 AM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Thu, Dec 07, 2017 at 11:23:26PM +0000, Al Viro wrote: >> On Fri, Dec 08, 2017 at 12:03:07AM +0100, Vasyl Gomonovych wrote: >> > p could be NULL and passing into PTR_ERR >> > >> > Signed-off-by: Vasyl Gomonovych <gomonovych@gmail.com> >> > --- >> > fs/seq_file.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/fs/seq_file.c b/fs/seq_file.c >> > index 4be761c..8b700b9 100644 >> > --- a/fs/seq_file.c >> > +++ b/fs/seq_file.c >> > @@ -262,8 +262,8 @@ ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t *ppos) >> > size_t offs = m->count; >> > loff_t next = pos; >> > p = m->op->next(m, p, &next); >> > - if (!p || IS_ERR(p)) { >> > - err = PTR_ERR(p); >> > + if (IS_ERR(p)) { >> > + err = (!p ? -EFAULT : PTR_ERR(p)); >> >> What does it fix, if I might ask? And while we are at it, would >> you mind explaining the reasoning behind that change? Or, say, >> testing done to it... > > While we are at it, where has that -EFAULT come from? And how > would it be ever reached, seeing that IS_ERR(NULL) is false? -- Доброї вам пори дня. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-08 7:54 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-12-07 23:03 [PATCH] fs/seq_file: Fix warning of passing zero to 'PTR_ERR' Vasyl Gomonovych 2017-12-07 23:20 ` Matthew Wilcox 2017-12-07 23:23 ` Al Viro 2017-12-07 23:26 ` Al Viro 2017-12-08 7:54 ` Gomonovych, Vasyl
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).