Skip to content

Commit

Permalink
Auto merge of #427 - oconnor663:pipe2, r=fiveop
Browse files Browse the repository at this point in the history
call pipe2 directly on Linux

A first shot at fixing #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?
  • Loading branch information
homu committed Sep 17, 2016
2 parents 3c8ff4a + 1d262d0 commit 6ea8f7f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 30 additions & 9 deletions src/unistd.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 6ea8f7f

Please sign in to comment.