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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On May 20, 2017, at 3:49 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> 
> We have never used this "err = PTR_ERR(p);" assignment and it annoys
> static checkers.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> 
> 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: signature.asc
Description: Message signed with OpenPGP


[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux