cvs commit: src/sys/kern kern_descrip.c
ed at 80386.nl
Thu May 29 08:26:22 UTC 2008
* Robert Watson <rwatson at 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
>> 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
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:
@@ -423,7 +423,8 @@
tmp = arg;
- error = do_dup(td, DUP_FIXED, fd, tmp, td->td_retval);
+ error = do_dup(td, DUP_FIXED|DUP_FCNTL, fd, tmp,
@@ -452,14 +452,6 @@
-.Dv F_DUP2FD ,
-is not a valid file descriptor.
.Dv F_SETLKW ,
@@ -502,6 +494,8 @@
is negative or greater than the maximum allowable number
Ed Schouten <ed at 80386.nl>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 195 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/cvs-src/attachments/20080529/8720e2bb/attachment.pgp
More information about the cvs-src