From owner-cvs-all@FreeBSD.ORG Thu May 29 08:26:22 2008 Return-Path: Delivered-To: cvs-all@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 67471106566C; Thu, 29 May 2008 08:26:22 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: from palm.hoeg.nl (mx0.hoeg.nl [IPv6:2001:610:652::211]) by mx1.freebsd.org (Postfix) with ESMTP id 27E548FC17; Thu, 29 May 2008 08:26:21 +0000 (UTC) (envelope-from ed@hoeg.nl) Received: by palm.hoeg.nl (Postfix, from userid 1000) id A9F911CC1E; Thu, 29 May 2008 10:24:33 +0200 (CEST) Date: Thu, 29 May 2008 10:24:33 +0200 From: Ed Schouten To: Robert Watson Message-ID: <20080529082433.GV64397@hoeg.nl> References: <200805282025.m4SKPJgY097901@repoman.freebsd.org> <20080529085549.J39873@fledge.watson.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="1gsfN/+pS0/2Ta7u" Content-Disposition: inline In-Reply-To: <20080529085549.J39873@fledge.watson.org> User-Agent: Mutt/1.5.18 (2008-05-17) Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_descrip.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 May 2008 08:26:22 -0000 --1gsfN/+pS0/2Ta7u Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable * Robert Watson wrote: > > On Wed, 28 May 2008, Ed Schouten wrote: > >> Remove redundant checks from fcntl()'s F_DUPFD. >> >> Right now we perform some of the checks inside the fcntl()'s F_DUPFD >> operation twice. We first validate the `fd' argument. When finished, >> we validate the `arg' argument. These checks are also performed inside >> do_dup(). >> >> The reason we need to do this, is because fcntl() should return differe= nt >> errno's when the `arg' argument is out of bounds (EINVAL instead of >> EBADF). To prevent the redundant locking of the PROC_LOCK and >> FILEDESC_SLOCK, patch do_dup() to support the error semantics required >> by fcntl(). > > This sounds like a good candidate for a regression test -- do we have a = =20 > dup/dup2/F_DUPFD/F_DUP2FD test? If not, perhaps we should, in light of= =20 > the opportunity for further bugs and regressions. It looks like we already have regression tests for dup/dup2/etc -- kern_descrip.c 1.325 mentions them. I saw FreeBSD also implements F_DUP2FD (which is a non-standard extension). Right now this command returns EBADF when you do the following: fcntl(0, F_DUP2FD, -1); // below zero fcntl(0, F_DUP2FD, 1000000); // too high This is exactly the same as what dup2() does, but is inconsistent with fcntl() in general. EBADF should be returned if the `fd' argument is invalid. It should not apply to the argument. We could consider applying the following patch: --- sys/kern/kern_descrip.c +++ sys/kern/kern_descrip.c @@ -423,7 +423,8 @@ =20 case F_DUP2FD: tmp =3D arg; - error =3D do_dup(td, DUP_FIXED, fd, tmp, td->td_retval); + error =3D do_dup(td, DUP_FIXED|DUP_FCNTL, fd, tmp, + td->td_retval); break; =20 case F_GETFD: --- lib/libc/sys/fcntl.2 +++ lib/libc/sys/fcntl.2 @@ -452,14 +452,6 @@ The argument .Fa cmd is -.Dv F_DUP2FD , -and -.Fa arg -is not a valid file descriptor. -.Pp -The argument -.Fa cmd -is .Dv F_SETLK or .Dv F_SETLKW , @@ -502,6 +494,8 @@ argument is .Dv F_DUPFD +or +.Dv F_DUP2FD and .Fa arg is negative or greater than the maximum allowable number Any comments? --=20 Ed Schouten WWW: http://80386.nl/ --1gsfN/+pS0/2Ta7u Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkg+aEEACgkQ52SDGA2eCwWpHQCeLuHarSIOeE6HJKASYXZs6EB6 Y2UAnRW3ArgqxdXLAkEdrEd7rmmNjdIU =DLG5 -----END PGP SIGNATURE----- --1gsfN/+pS0/2Ta7u--