On Thu, May 29, 2008 at 10:24 AM, Ed Schouten <[EMAIL PROTECTED]> wrote: > * 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?
Hello Ed, On Solaris, dup2 and fcntl(F_DUP2FD) are totally equivalent (they return the same errno). I have no strong feelings about this. Cheers, Antoine _______________________________________________ cvs-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/cvs-all To unsubscribe, send any mail to "[EMAIL PROTECTED]"
