* Robert Watson <[EMAIL PROTECTED]> 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 different >> 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 > dup/dup2/F_DUPFD/F_DUP2FD test? If not, perhaps we should, in light of > 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 @@
case F_DUP2FD:
tmp = arg;
- error = do_dup(td, DUP_FIXED, fd, tmp, td->td_retval);
+ error = do_dup(td, DUP_FIXED|DUP_FCNTL, fd, tmp,
+ td->td_retval);
break;
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?
--
Ed Schouten <[EMAIL PROTECTED]>
WWW: http://80386.nl/
pgpArvEwrUbeb.pgp
Description: PGP signature
