GNOME Bugzilla – Bug 697024
use dup2() instead of unportable dup3()
Last modified: 2013-04-08 12:11:21 UTC
It seems that the usage of dup3() is not needed and dup2() suffices. This would fix some breakage seen on non-Linux as dup3() is a GLIBC-extension, not defined in POSIX. I have not noticed any regressions in runtime when using dup2(), so would it be ok to push this patch?
Created attachment 240281 [details] [review] Use dup2() instead of dup3()
Review of attachment 240281 [details] [review]: ::: src/terminal-screen.c @@ +1347,3 @@ errno = 0; do { + fd = dup2 (fds[idx], target_fd, 0 /* no FD_CLOEXEC */); dup2 only takes 2 arguments, not 3. Which is why dup3 is used here. Does dup2 copy the flags as well? The new FD really needs to be not-FD_CLOEXEC. So if there's no guarantee in the API, you'll need to use fcntl to make sure it's not-FD_CLOEXEC. (Also what platform we should care about doesn't have dup3 ?)
Since dup3() is a GLIBC-extension, it doesn't work on all non-Linux/CYGWIN platforms, such as *BSD, Solaris, etc. From the OpenBSD dup2(2) manpage: The object referenced by the descriptor does not distinguish between oldd and newd in any way. Thus if newd and oldd are duplicate references to an open file, read(2), write(2) and lseek(2) calls all move a single pointer into the file, and append mode, non-blocking I/O and asynchronous I/O options are shared between the references. If a separate pointer into the file is desired, a different object reference to the file must be obtained by issuing an additional open(2) call. The close-on-exec flag on the new file descriptor is unset. I haven't tested it yet, but from the description one could say that it does copy the flags indeed ("...does not distinguish between oldd and newd in any way."). The source of sys_dup2 in OpenBSD is here: http://www.openbsd.org/cgi-bin/cvsweb/src/sys/kern/kern_descrip.c?rev=HEAD;content-type=text%2Fplain Otherwise, would a construct like this be desirable? (untested though). } else { /* Now we know that target_fd can be safely overwritten. */ errno = 0; do { fd = fcntl(fds[idx], F_DUPFD, target_fd); } while (fd == -1 && errno == EINTR); if (fd != target_fd) _exit (127); }
Let's just do #ifdef __linux use dup3 #else use dup2 get flags if (flags & FD_CLOEXEC) _exit(127); #endif
Created attachment 240928 [details] [review] Only use dup3 on Linux
The patch doesn't work; you can just test fd & FD_CLOEXEC ! Anyway, I committed a different patch to master and 3-8.
Thanks :)