Date: Thu, 29 May 2008 10:24:33 +0200 From: Ed Schouten <ed@80386.nl> To: Robert Watson <rwatson@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/kern kern_descrip.c Message-ID: <20080529082433.GV64397@hoeg.nl> In-Reply-To: <20080529085549.J39873@fledge.watson.org> References: <200805282025.m4SKPJgY097901@repoman.freebsd.org> <20080529085549.J39873@fledge.watson.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--1gsfN/+pS0/2Ta7u Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable * Robert Watson <rwatson@FreeBSD.org> 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 <ed@80386.nl> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080529082433.GV64397>