Cleaning up FILE in stdio..

[John Baldwin]

On Wednesday 27 February 2008 12:26:26 am Garrett Wollman wrote:
> In article <200802262355.16519.jhb at freebsd.org>,
> John Baldwin <jhb at freebsd.org> writes:
> >On Tuesday 26 February 2008 05:51:07 pm Garrett Wollman wrote:
> 
> >+	/*
> >+	 * File descriptors are a full int, but _file is only a short.
> >+	 * If we get a valid file descriptor that is greater than
> >+	 * SHRT_MAX, then the fd will get sign-extended into an
> >+	 * invalid file descriptor.  Handle this case by failing the
> >+	 * open.
> >+	 */
> >+	if (fd > SHRT_MAX) {
> >+		errno = EINVAL;
> >+		return (NULL);
> >+	}
> >+
> 
> Please, please, please, whatever you do, don't add Yet Another
> Overloaded Meaning for [EINVAL].  Use [EMFILE] instead, which is
> defined to have the precise meaning desired here.  For extra credit,
> fix the various places {STREAM_MAX} is defined to take this limit into
> account.  I think the following may be all that is required (beware
> xterm cut-and-paste screwage):

I used EINVAL rather than ENOMEM for fdopen() since fdopen() is documented to 
return errors for fcntl(2) and one of the EINVAL's for fcntl(2) is:

     [EINVAL]           The cmd argument is F_DUPFD and arg is negative or
                        greater than the maximum allowable number (see
                        getdtablesize(2)).

I avoided EMFILE in all 3 cases as it struck me as being not really true (an 
app would find the rlimit higher than the current fd for example).  Also, 
EMFILE doesn't really make sense from fdopen() at all.  You've already opened 
the fd, so you know you can't run out of fd's.

> Index: lib/libc/gen/sysconf.c
> ===================================================================
> RCS file: /home/ncvs/src/lib/libc/gen/sysconf.c,v
> retrieving revision 1.20
> diff -u -r1.20 sysconf.c
> --- lib/libc/gen/sysconf.c	17 Nov 2002 08:54:29 -0000	1.20
> +++ lib/libc/gen/sysconf.c	27 Feb 2008 05:23:24 -0000
> @@ -105,7 +105,6 @@
> 		mib[1] = KERN_NGROUPS;
> 		break;
> 	case _SC_OPEN_MAX:
> -	case _SC_STREAM_MAX:	/* assume fds run out before memory does */
> 		if (getrlimit(RLIMIT_NOFILE, &rl) != 0)
> 			return (-1);
> 		if (rl.rlim_cur == RLIM_INFINITY)
> @@ -115,6 +114,25 @@
> 			return (-1);
> 		}
> 		return ((long)rl.rlim_cur);
> +	case _SC_STREAM_MAX:
> +		if (getrlimit(RLIMIT_NOFILE, &rl) != 0)
> +			return (-1);
> +		if (rl.rlim_cur == RLIM_INFINITY)
> +			return (-1);
> +		if (rl.rlim_cur > LONG_MAX) {
> +			errno = EOVERFLOW;
> +			return (-1);
> +		}
> +		/*
> +		 * struct __sFILE currently has a limitation that
> +		 * file descriptors must fit in a signed short.
> +		 * This doesn't precisely capture the letter of POSIX
> +		 * but approximates the spirit.
> +		 */
> +		if (rl.rlim_cur > SHRT_MAX)
> +			return (SHRT_MAX);
> +
> +		return ((long)rl.rlim_cur);
> 	case _SC_JOB_CONTROL:
> 		return (_POSIX_JOB_CONTROL);
> 	case _SC_SAVED_IDS:
> 
> 
> -GAWollman

Ah, thanks.

-- 
John Baldwin