Skip to content

Commit

Permalink
Merge #1293
Browse files Browse the repository at this point in the history
1293: Mark nix::unistd::fork as unsafe. r=asomers a=vi

Fix tests. No change in documentation.

Resolves #1030.

Don't forget to bump major version number to `0.19`.

Co-authored-by: Vitaly _Vi Shukela <vi0oss@gmail.com>
  • Loading branch information
bors[bot] and vi authored Oct 3, 2020
2 parents fe0aa23 + 42172a6 commit 8e3789d
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 13 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ This project adheres to [Semantic Versioning](http://semver.org/).
(#[1284](https://github.com/nix-rust/nix/pull/1284))
- Changed unistd::{execv,execve,execvp,execvpe,fexecve,execveat} to take both `&[&CStr]` and `&[CString]` as its list argument(s).
(#[1278](https://github.com/nix-rust/nix/pull/1278))
- Made `unistd::fork` an unsafe funtion, bringing it in line with [libstd's decision](https://github.com/rust-lang/rust/pull/58059).
(#[1293](https://github.com/nix-rust/nix/pull/1293))
### Fixed
### Removed

Expand Down
6 changes: 3 additions & 3 deletions src/unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ impl ForkResult {
/// ```no_run
/// use nix::unistd::{fork, ForkResult};
///
/// match fork() {
/// match unsafe{fork()} {
/// Ok(ForkResult::Parent { child, .. }) => {
/// println!("Continuing execution in parent process, new child has pid: {}", child);
/// }
Expand Down Expand Up @@ -230,9 +230,9 @@ impl ForkResult {
///
/// [async-signal-safe]: http://man7.org/linux/man-pages/man7/signal-safety.7.html
#[inline]
pub fn fork() -> Result<ForkResult> {
pub unsafe fn fork() -> Result<ForkResult> {
use self::ForkResult::*;
let res = unsafe { libc::fork() };
let res = libc::fork();

Errno::result(res).map(|res| match res {
0 => Child,
Expand Down
4 changes: 2 additions & 2 deletions test/sys/test_ptrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ fn test_ptrace_cont() {
return;
}

match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => {
ptrace::traceme().unwrap();
// As recommended by ptrace(2), raise SIGTRAP to pause the child
Expand Down Expand Up @@ -132,7 +132,7 @@ fn test_ptrace_syscall() {

let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => {
ptrace::traceme().unwrap();
// first sigstop until parent is ready to continue
Expand Down
2 changes: 1 addition & 1 deletion test/sys/test_uio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ fn test_process_vm_readv() {
let mut vector = vec![1u8, 2, 3, 4, 5];

let (r, w) = pipe().unwrap();
match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Parent { child } => {
close(w).unwrap();
// wait for child
Expand Down
8 changes: 4 additions & 4 deletions test/sys/test_wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn test_wait_signal() {
let _ = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

// Safe: The child only calls `pause` and/or `_exit`, which are async-signal-safe.
match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => {
pause();
unsafe { _exit(123) }
Expand All @@ -28,7 +28,7 @@ fn test_wait_exit() {
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

// Safe: Child only calls `_exit`, which is async-signal-safe.
match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => unsafe { _exit(12); },
Parent { child } => {
assert_eq!(waitpid(child, None), Ok(WaitStatus::Exited(child, 12)));
Expand All @@ -48,7 +48,7 @@ fn test_waitstatus_from_raw() {
fn test_waitstatus_pid() {
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

match fork().unwrap() {
match unsafe{fork()}.unwrap() {
Child => unsafe { _exit(0) },
Parent { child } => {
let status = waitpid(child, None).unwrap();
Expand Down Expand Up @@ -98,7 +98,7 @@ mod ptrace {
require_capability!(CAP_SYS_PTRACE);
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => ptrace_child(),
Parent { child } => ptrace_parent(child),
}
Expand Down
6 changes: 3 additions & 3 deletions test/test_unistd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn test_fork_and_waitpid() {
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

// Safe: Child only calls `_exit`, which is signal-safe
match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => unsafe { _exit(0) },
Parent { child } => {
// assert that child was created and pid > 0
Expand Down Expand Up @@ -60,7 +60,7 @@ fn test_wait() {
let _m = crate::FORK_MTX.lock().expect("Mutex got poisoned by another test");

// Safe: Child only calls `_exit`, which is signal-safe
match fork().expect("Error: Fork Failed") {
match unsafe{fork()}.expect("Error: Fork Failed") {
Child => unsafe { _exit(0) },
Parent { child } => {
let wait_status = wait();
Expand Down Expand Up @@ -302,7 +302,7 @@ macro_rules! execve_test_factory(
// Safe: Child calls `exit`, `dup`, `close` and the provided `exec*` family function.
// NOTE: Technically, this makes the macro unsafe to use because you could pass anything.
// The tests make sure not to do that, though.
match fork().unwrap() {
match unsafe{fork()}.unwrap() {
Child => {
// Make `writer` be the stdout of the new process.
dup2(writer, 1).unwrap();
Expand Down

0 comments on commit 8e3789d

Please sign in to comment.