[PATCH] fs/seq_file: remove some dead code

[PATCH] fs/seq_file: remove some dead code

[Dan Carpenter]

We have never used this "err = PTR_ERR(p);" assignment and it annoys
static checkers.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/fs/seq_file.c b/fs/seq_file.c
index 13e8c092d4d2..d6f82ce288f4 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -261,10 +261,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 (!p || IS_ERR(p))
 			break;
-		}
 		err = m->op->show(m, p);
 		if (seq_has_overflowed(m) || err) {
 			m->count = offs;

Re: [PATCH] fs/seq_file: remove some dead code

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1761 bytes --]

On May 20, 2017, at 3:49 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> 
> We have never used this "err = PTR_ERR(p);" assignment and it annoys
> static checkers.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/fs/seq_file.c b/fs/seq_file.c
> index 13e8c092d4d2..d6f82ce288f4 100644
> --- a/fs/seq_file.c
> +++ b/fs/seq_file.c
> @@ -261,10 +261,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 (!p || IS_ERR(p))
> 			break;
> -		}
> 		err = m->op->show(m, p);
> 		if (seq_has_overflowed(m) || err) {
> 			m->count = offs;

I'm guessing the static analysis is reporting that "err" is unused, because
it is clobbered after the end of the loop?

			if (likely(err <= 0))
				break;
		}
		pos = next;
	}
	m->op->stop(m, p);
	n = min(m->count, size);
	err = copy_to_user(buf, m->buf, n);
	if (err)
		goto Efault;

Unfortunately, this code dates back to the primordial "1da177e4" commit at
the start of Git history, so history to see how it got into this state.

Presumably, copy_to_user() is called unconditionally to return all data read
so far to userspace, and in that case "err" doesn't matter - something was
returned to userspace.  I was thinking that copy_to_user() should be skipped
if "n == 0", so that the original "err" value is returned, but it may be that
this is never true in the "Fill:" case (ugh, upper-case labels).

	n = min(m->count, size);
	if (n == 0)
		goto Done;

The logic isn't very clear here, so it isn't clear what the right change is.

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]