On Thu, 29 May 2008, Ed Schouten wrote:

* Robert Watson <[EMAIL PROTECTED]> wrote:
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.

No, EBADF applies to both the open fd and the new fd for dup2() (since
that is what dup2() did initially and has become Standard).  F_DUP2FD
is apparently supposed to be equivalent to dup2(), so it must be bug-for
bug equivalent.

dup2() was broken in rev.1.219.  It actually returns EMFILE if the new
fd is too high.  It remains unbroken and returns EBADF if the new fd
is below zero, but this gives another inconsistency.  I think F_DUP2FD
inherits these bugs.  F_DUPFD consistently returns EINVAL if the new
fd is either too high or below zero.  More details in previously-sent
private mail.

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:

Style bug (missing spaces around binary operator).

--- 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?

I think F_DUP2FD and its man page are as intended and don't need changing.
But dup2()'s man page needs changes to spell out the details of the
differences between the error handling of dup() and dup2() (it is shared
with dup()'s man page but no differences are mentioned).  Comparing with
POSIX shows that the main differences are:
- [EMFILE] doesn't apply to dup2(), since the new fd is available unless
  it is out of bounds, and the error for out of bounds is EBADF
- for dup2(), it is not an error for the new fd to be "inactive",

and that the main bugs which aren't differences are:

- "active" is a bad way of saying "open"
- "valid" is a fuzzy way of saying "not (negative or >= {OPEN_MAX})".
  fcntl.2 already says this better (see above quote; in the unquoted
  part it spells {OPEN_MAX} as "see getdtablesize(2)").  I think
  FreeBSD man pages should explicitly refer to intro(2) for the the
  details of this, but intro(2) is about 20 years out of date here --
  it says that releases have a fixed limit of 64 and refers to
  getdtablesize(2) and never mentions {OPEN_MAX}.
- [EINTR] is not described.  I'm not sure if it applies to the dup()
  family when syscalls are restarted, but it applies when they are not
  and is mentioned for open(2).

Bruce
_______________________________________________
cvs-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/cvs-all
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to