Because dup3() with the additional flags argument would be useful in general, here is an implementation for FreeBSD. Some other UNIX style systems (at least NetBSD and Linux) already have a similar function. So, there is also the enhanced portability point of view driving this. Fix: Find a patch attached. Patch attached with submission follows: How-To-Repeat: No actual problem. Only a nifty add-on feature and enhanced portability.
PR kern/176233: > [dup3 implementation based on F_DUP2FD_CLOEXEC] The dup3() function appears to be used in the wild and there is a gnulib replacement for it, even though it seems of rather limited use compared to fcntl(F_DUPFD_CLOEXEC). Most uses of dup2() are for passing fd 0, 1 or 2 to a process so that the close-on-exec flag should be clear. I think glibc's [EINVAL] on oldfd == newfd actually makes some sense as it forces the programmer to separate clearly the cases where oldfd != newfd and oldfd == newfd. http://austingroupbugs.net/view.php?id=411 also proposes requiring it. The O_NONBLOCK and O_NOSIGPIPE flags appear contrary to the idea of dup2/dup3 since they affect the underlying object and not just the new file descriptor like O_CLOEXEC does. Unknown/unsupported flags should indeed [EINVAL] so we have a chance to add new flags later. -- Jilles Tjoelker
Quoting Jilles Tjoelker: > > PR kern/176233: > > [dup3 implementation based on F_DUP2FD_CLOEXEC] > > The dup3() function appears to be used in the wild and there is a gnulib > replacement for it, even though it seems of rather limited use compared > to fcntl(F_DUPFD_CLOEXEC). Most uses of dup2() are for passing fd 0, 1 > or 2 to a process so that the close-on-exec flag should be clear. > > I think glibc's [EINVAL] on oldfd == newfd actually makes some sense as > it forces the programmer to separate clearly the cases where oldfd != > newfd and oldfd == newfd. http://austingroupbugs.net/view.php?id=411 > also proposes requiring it. This is a kind of a philosophical choise. Do we expect the programmer to pass two equal file descriptors intentionally or not? The Linux community and the Austin group have taken the view in which they do not trust the programmer to do it intentionally. The NetBSD way is to rely on the programmer to use the trick by choise, not by accident. Personally I rather like the latter approach. ;-) Also simple good style favors sticking to similar behavior as the existing dup2() while the "big academic problem" Linux has been solving with their deviant behavior is in fact more of a paper tiger. Anyhow were the large majority to prefer the pointlessly restrictive (in my mind) Linux approach over the NetBSD approach and over consistency with the existing dup2(), you have the necessary check included in my patch between "#if 0/#endif". Any time this sort of discussion about which school of thought to follow breaks up I quite miss the old ConvexOS "warp" concept. It would allow us to choose both ways at the same time. In ConvexOS warp was part of the process features which were inherited by the children, and the setting allowed various parts of the system (system calls, library functions, etc.) to choose which tradition and which school of thouht to follow. One just had to set a warp variable on the command line and the shell set that for the child processes using the setwarp() system call just before exec*(). In this case I would personally like to enclose the check against equal file descriptors within a special warp check and be over with it... if (getwarp() == WARP_LINUX) { if (oldd == newd) { return (EINVAL); } } FreeBSD just does not support the warp concept - at least not yet. OTOH adding the warp feature might actually help with the linuxulator and other emulation efforts. > The O_NONBLOCK and O_NOSIGPIPE flags appear contrary to the idea of > dup2/dup3 since they affect the underlying object and not just the new > file descriptor like O_CLOEXEC does. Right, I checked the current FreeBSD implementations of O_NONBLOCK and SO_NOSIGPIPE, which is the closest thing FreeBSD has to O_NOSIGPIPE. They seem to be implemented as flags in the file pointer structure and in the socket structure, not as part of the descriptor. The first question of course is whether the flags really have to be implemented this way, if the interpretation gives raise to problems? The second equally obvious question is, what prevents the programmer from setting those flags on the resulting new descriptor anyhow right after calling the existing dup2() and still affecting also the old descriptor in the process? Since the most common action to the old descriptor after dup2() is anyhow close() in case (oldfd != newfd), the side effect to the old descriptor should be mostly irrelevant. At the same time the added convenience to the programmer might be quite beneficial in avoiding situations in which one simply forgets to set such flags when they would be needed. Compactness and readability of the code is also enhanced when one does not have to write separate function/system calls after dup2() to get the same effect. A technical limitation might simply not be the proper solution to something which would be better served by adding a serious warning in the manual page saying roughly... "Do not call dup*() and then expect calling lseek() or changing flags on one of the multiple descriptors to an object not to affect the behavior when using any of the other descriptors to the same object. The dup*() family only adds handles to an object. It does not make copies of the underlying object. Though you gain multiple descriptors you still have only one object. If you change an object through any of the descriptors, the change will be visible also when using the object through any of the other descriptors." > Unknown/unsupported flags should indeed [EINVAL] so we have a chance to > add new flags later. ;-) Cheers, // jau .--- ..- -.- -.- .- .- .-.-.- ..- -.- -.- --- -. . -. / Jukka A. Ukkonen, Oxit Ltd, Finland /__ M.Sc. (sw-eng & cs) (Phone) +358-500-606-671 / Internet: Jukka.Ukkonen(a)Oxit.Fi / Internet: jau(a)iki.fi v .--- .- ..- ...-.- .. -.- .. .-.-.- ..-. .. + + + + My opinions are mine and mine alone, not my employers. + + + +
On Wed, Feb 20, 2013 at 11:13:14AM +0200, Jukka A. Ukkonen wrote: > Quoting Jilles Tjoelker: > > PR kern/176233: > > > [dup3 implementation based on F_DUP2FD_CLOEXEC] > > The dup3() function appears to be used in the wild and there is a gnulib > > replacement for it, even though it seems of rather limited use compared > > to fcntl(F_DUPFD_CLOEXEC). Most uses of dup2() are for passing fd 0, 1 > > or 2 to a process so that the close-on-exec flag should be clear. > > I think glibc's [EINVAL] on oldfd == newfd actually makes some sense as > > it forces the programmer to separate clearly the cases where oldfd != > > newfd and oldfd == newfd. http://austingroupbugs.net/view.php?id=411 > > also proposes requiring it. > This is a kind of a philosophical choise. > Do we expect the programmer to pass two equal file descriptors > intentionally or not? > The Linux community and the Austin group have taken the view in > which they do not trust the programmer to do it intentionally. > The NetBSD way is to rely on the programmer to use the trick > by choise, not by accident. Personally I rather like the latter > approach. ;-) > Also simple good style favors sticking to similar behavior as the > existing dup2() while the "big academic problem" Linux has been > solving with their deviant behavior is in fact more of a paper tiger. > Anyhow were the large majority to prefer the pointlessly restrictive > (in my mind) Linux approach over the NetBSD approach and over > consistency with the existing dup2(), you have the necessary check > included in my patch between "#if 0/#endif". Some more rationale from the Linux/glibc side about this: http://www.cygwin.com/ml/libc-ports/2011-09/msg00009.html Basically, if x != y, dup3(x, y, O_CLOEXEC) ensures y is set CLOEXEC and dup3(x, y, 0) ensures y is set not CLOEXEC. This breaks down if x == y. -- Jilles Tjoelker
Responsible Changed From-To: freebsd-bugs->jilles I plan to commit this because it seems dup3() is a bit more popular than fcntl(F_DUP2FD_CLOEXEC). This may be after the other CLOEXEC changes. Regarding use, it appears that APR can use this, and I even used F_DUP2FD_CLOEXEC myself in freopen().
I just noticed that the patch included in this ticket is slightly in error when setting up the manual pages. This change... --- lib/libc/sys/Makefile.inc.orig 2013-02-16 10:34:41.000000000 +0200 +++ lib/libc/sys/Makefile.inc 2013-02-16 22:09:12.000000000 +0200 @@ -132,7 +132,7 @@ MLINKS+=clock_gettime.2 clock_getres.2 clock_gettime.2 clock_settime.2 MLINKS+=cpuset.2 cpuset_getid.2 cpuset.2 cpuset_setid.2 MLINKS+=cpuset_getaffinity.2 cpuset_setaffinity.2 -MLINKS+=dup.2 dup2.2 +MLINKS+=dup.2 dup2.2 dup3.2 MLINKS+=execve.2 fexecve.2 MLINKS+=extattr_get_file.2 extattr.2 \ extattr_get_file.2 extattr_delete_fd.2 \ should be like this... --- lib/libc/sys/Makefile.inc.orig 2013-02-16 10:34:41.000000000 +0200 +++ lib/libc/sys/Makefile.inc 2013-02-16 22:09:12.000000000 +0200 @@ -132,7 +132,7 @@ MLINKS+=clock_gettime.2 clock_getres.2 clock_gettime.2 clock_settime.2 MLINKS+=cpuset.2 cpuset_getid.2 cpuset.2 cpuset_setid.2 MLINKS+=cpuset_getaffinity.2 cpuset_setaffinity.2 -MLINKS+=dup.2 dup2.2 +MLINKS+=dup.2 dup2.2 dup.2 dup3.2 MLINKS+=execve.2 fexecve.2 MLINKS+=extattr_get_file.2 extattr.2 \ extattr_get_file.2 extattr_delete_fd.2 \ So, the new MLINKS setting was missing one instance of "dup.2" which will create a mess in the man2 directory, if applied as is. For some reason I had saved an older version of the patch file and when sending the change ticket I apparently managed to attach the older and broken file. Cheers, --jau
Author: jilles Date: Fri Aug 16 13:10:30 2013 New Revision: 254409 URL: http://svnweb.freebsd.org/changeset/base/254409 Log: Add dup3(), based on F_DUP2FD and F_DUP2FD_CLOEXEC fcntls. I removed functionality not proposed for POSIX in Austin group issue #411. A man page (my own) and test cases will follow in later commits. PR: 176233 Submitted by: Jukka Ukkonen Added: head/lib/libc/gen/dup3.c (contents, props changed) Modified: head/include/unistd.h head/lib/libc/gen/Makefile.inc head/lib/libc/gen/Symbol.map Modified: head/include/unistd.h ============================================================================== --- head/include/unistd.h Fri Aug 16 12:25:02 2013 (r254408) +++ head/include/unistd.h Fri Aug 16 13:10:30 2013 (r254409) @@ -493,6 +493,7 @@ const char * int crypt_set_format(const char *); int des_cipher(const char *, char *, long, int); int des_setkey(const char *key); +int dup3(int, int, int); int eaccess(const char *, int); void endusershell(void); int exect(const char *, char * const *, char * const *); Modified: head/lib/libc/gen/Makefile.inc ============================================================================== --- head/lib/libc/gen/Makefile.inc Fri Aug 16 12:25:02 2013 (r254408) +++ head/lib/libc/gen/Makefile.inc Fri Aug 16 13:10:30 2013 (r254409) @@ -31,6 +31,7 @@ SRCS+= __getosreldate.c \ disklabel.c \ dlfcn.c \ drand48.c \ + dup3.c \ elf_utils.c \ erand48.c \ err.c \ Modified: head/lib/libc/gen/Symbol.map ============================================================================== --- head/lib/libc/gen/Symbol.map Fri Aug 16 12:25:02 2013 (r254408) +++ head/lib/libc/gen/Symbol.map Fri Aug 16 13:10:30 2013 (r254409) @@ -383,6 +383,7 @@ FBSD_1.2 { FBSD_1.3 { clock_getcpuclockid; dirfd; + dup3; fdlopen; __FreeBSD_libc_enter_restricted_mode; getcontextx; Added: head/lib/libc/gen/dup3.c ============================================================================== --- /dev/null 00:00:00 1970 (empty, because file is newly added) +++ head/lib/libc/gen/dup3.c Fri Aug 16 13:10:30 2013 (r254409) @@ -0,0 +1,59 @@ +/*- + * Copyright (c) 2012 Jukka A. Ukkonen + * All rights reserved. + * + * This software was developed by Jukka Ukkonen for FreeBSD. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +#include <sys/cdefs.h> +__FBSDID("$FreeBSD$"); + +#include "namespace.h" +#include <unistd.h> +#include <fcntl.h> +#include <errno.h> +#include "un-namespace.h" + +int +__dup3(int oldfd, int newfd, int flags) +{ + int how; + + if (oldfd == newfd) { + errno = EINVAL; + return (-1); + } + + if (flags & ~O_CLOEXEC) { + errno = EINVAL; + return (-1); + } + + how = (flags & O_CLOEXEC) ? F_DUP2FD_CLOEXEC : F_DUP2FD; + + return (_fcntl(oldfd, how, newfd)); +} + +__weak_reference(__dup3, dup3); +__weak_reference(__dup3, _dup3); _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
State Changed From-To: open->patched Added to 10-current, thanks.
Fixed in 10.0 and newer branches, uninteresting for older branches that probably will not see another release.