diff options
author | Homu <homu@barosl.com> | 2016-09-18 04:14:05 +0900 |
---|---|---|
committer | Homu <homu@barosl.com> | 2016-09-18 04:14:05 +0900 |
commit | 6ea8f7f91c65aa4d20a95dc79995904c374d7a88 (patch) | |
tree | de410b797fe4ce2f92fefabe46dc29f72d4cc4a3 | |
parent | 3c8ff4ab916132011c64c523cab78cbdf9a2a13e (diff) | |
parent | 1d262d0541c729df94351b73eef0957785d28d85 (diff) | |
download | nix-6ea8f7f91c65aa4d20a95dc79995904c374d7a88.zip |
Auto merge of #427 - oconnor663:pipe2, r=fiveop
call pipe2 directly on Linux
A first shot at fixing https://github.com/nix-rust/nix/issues/414. This approach keeps the old implementation for platforms other than `notbsd`, because `libc` only exposes `pipe2` in the `notbsd` module.
I've tested this by hand on my Linux machine in a couple ways:
- Create a toy program that opens a pipe and passes it to `cat`, which hags if `O_CLOEXEC` doesn't get set properly. Confirm that it doesn't hang, but that it does if I pass `0` as the flags to `libc::pipe2`.
- Delete the new implementation entirely, and delete the `cfg` guards on the old implementation, and confirm that above is still true.
I haven't actually tested a cross compilation build though. Is there a standard way to do that?
-rw-r--r-- | CHANGELOG.md | 5 | ||||
-rw-r--r-- | src/unistd.rs | 39 |
2 files changed, 35 insertions, 9 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4238ed74..3f8f54de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +### Changed +- `pipe2` now calls `libc::pipe2` where available. Previously it was emulated + using `pipe`, which meant that setting `O_CLOEXEC` was not atomic. + ([#427](https://github.com/nix-rust/nix/pull/427)) + ## [0.7.0] 2016-09-09 ### Added diff --git a/src/unistd.rs b/src/unistd.rs index 74d74c9c..1b2175ea 100644 --- a/src/unistd.rs +++ b/src/unistd.rs @@ -1,8 +1,8 @@ //! Standard symbolic constants and types //! use {Errno, Error, Result, NixPath}; -use fcntl::{fcntl, OFlag, O_NONBLOCK, O_CLOEXEC, FD_CLOEXEC}; -use fcntl::FcntlArg::{F_SETFD, F_SETFL}; +use fcntl::{fcntl, OFlag, O_CLOEXEC, FD_CLOEXEC}; +use fcntl::FcntlArg::F_SETFD; use libc::{self, c_char, c_void, c_int, c_uint, size_t, pid_t, off_t, uid_t, gid_t, mode_t}; use std::mem; use std::ffi::{CString, CStr, OsString}; @@ -360,21 +360,42 @@ pub fn pipe() -> Result<(RawFd, RawFd)> { } } +// libc only defines `pipe2` in `libc::notbsd`. +#[cfg(any(target_os = "linux", + target_os = "android", + target_os = "emscripten"))] pub fn pipe2(flags: OFlag) -> Result<(RawFd, RawFd)> { - unsafe { - let mut fds: [c_int; 2] = mem::uninitialized(); + let mut fds: [c_int; 2] = unsafe { mem::uninitialized() }; - let res = libc::pipe(fds.as_mut_ptr()); + let res = unsafe { libc::pipe2(fds.as_mut_ptr(), flags.bits()) }; - try!(Errno::result(res)); + try!(Errno::result(res)); + + Ok((fds[0], fds[1])) +} - try!(pipe2_setflags(fds[0], fds[1], flags)); +#[cfg(not(any(target_os = "linux", + target_os = "android", + target_os = "emscripten")))] +pub fn pipe2(flags: OFlag) -> Result<(RawFd, RawFd)> { + let mut fds: [c_int; 2] = unsafe { mem::uninitialized() }; - Ok((fds[0], fds[1])) - } + let res = unsafe { libc::pipe(fds.as_mut_ptr()) }; + + try!(Errno::result(res)); + + try!(pipe2_setflags(fds[0], fds[1], flags)); + + Ok((fds[0], fds[1])) } +#[cfg(not(any(target_os = "linux", + target_os = "android", + target_os = "emscripten")))] fn pipe2_setflags(fd1: RawFd, fd2: RawFd, flags: OFlag) -> Result<()> { + use fcntl::O_NONBLOCK; + use fcntl::FcntlArg::F_SETFL; + let mut res = Ok(0); if flags.contains(O_CLOEXEC) { |