svn commit: r250220 - head/sys/kern

[Bruce Evans]

> Log:
>  Fix FIONREAD on regular files.  The computed result was being ignored and
>  it was being passed down to VOP_IOCTL() where it promptly resulted in
>  ENOTTY due to a missing else for the past 8 years.  While here, use a
>  shared vnode lock while fetching the current file's size.

In another thread I complained to kib about using the bad style of not
returning as soon as possible, but instead sometimes returning and sometimes
falling through a complicated if-else ladder to get to the return at the
end.

> Modified: head/sys/kern/vfs_vnops.c
> ==============================================================================
> --- head/sys/kern/vfs_vnops.c	Fri May  3 18:58:37 2013	(r250219)
> +++ head/sys/kern/vfs_vnops.c	Fri May  3 19:08:58 2013	(r250220)
> @@ -1364,13 +1364,12 @@ vn_ioctl(fp, com, data, active_cred, td)
> 	case VREG:
> 	case VDIR:
> 		if (com == FIONREAD) {
> -			vn_lock(vp, LK_EXCLUSIVE | LK_RETRY);
> +			vn_lock(vp, LK_SHARED | LK_RETRY);
> 			error = VOP_GETATTR(vp, &vattr, active_cred);
> 			VOP_UNLOCK(vp, 0);
> 			if (!error)
> 				*(int *)data = vattr.va_size - fp->f_offset;

Old versions were missing the bug because they just returned here.

> -		}
> -		if (com == FIONBIO || com == FIOASYNC)	/* XXX */
> +		} else if (com == FIONBIO || com == FIOASYNC)	/* XXX */
> 			error = 0;

This depended on the FIONREAD clause always returning.

This was simpler since it just returned 0 instead of setting error and
falling through a not-so-complicated if-else ladder to the return
statement.  But old versions used the bad style of falling through
the default case for the VREG and VDIR cases.

> 		else
> 			error = VOP_IOCTL(vp, com, data, fp->f_flag,

Better yet, in the old versions the default case was intended to
return ENOTTY, but this was hacked out, so the VREG and VDIR cases
that didn't already return fell through the null default statement
into the VFIFO+VCHR+VBLK case that did essentially this ioctl,
because a few file systems like cd9660 have some magic ioctls.
So the old versions needed the earlier returns even more, to
avoid special cases on falling through.

The default case is now obfuscated by setting error to ENOTTY at the
start of the function.  After this fix, that setting applies only to
the default case.  The setting is missing the style bug if initializing
error in its declaration.  Only the initialization of vp has that bug.

Bruce