From owner-freebsd-arch@FreeBSD.ORG Wed Feb 27 18:30:29 2008 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3C1951065681 for ; Wed, 27 Feb 2008 18:30:29 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from speedfactory.net (mail.speedfactory.net [66.23.216.219]) by mx1.freebsd.org (Postfix) with ESMTP id D0BE38FC1B for ; Wed, 27 Feb 2008 18:30:28 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (unverified [66.23.211.162]) by speedfactory.net (SurgeMail 3.8s) with ESMTP id 233599867-1834499 for multiple; Wed, 27 Feb 2008 13:28:10 -0500 Received: from localhost.corp.yahoo.com (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.14.2/8.14.2) with ESMTP id m1RITrOh035092; Wed, 27 Feb 2008 13:30:04 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: Garrett Wollman Date: Wed, 27 Feb 2008 11:34:04 -0500 User-Agent: KMail/1.9.7 References: <200802262251.m1QMp7bV021709@hergotha.csail.mit.edu> <200802270526.m1R5QQT3024163@hergotha.csail.mit.edu> In-Reply-To: <200802270526.m1R5QQT3024163@hergotha.csail.mit.edu> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200802271134.04166.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Wed, 27 Feb 2008 13:30:05 -0500 (EST) X-Virus-Scanned: ClamAV 0.91.2/6010/Wed Feb 27 07:54:14 2008 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: arch@freebsd.org Subject: Re: Cleaning up FILE in stdio.. X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 27 Feb 2008 18:30:29 -0000 On Wednesday 27 February 2008 12:26:26 am Garrett Wollman wrote: > In article <200802262355.16519.jhb@freebsd.org>, > John Baldwin 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