Skip to content

Commit

Permalink
run: Process::new() should take &[Str] instead of &[~str] for args
Browse files Browse the repository at this point in the history
Fixes #7928.
  • Loading branch information
lilyball committed Aug 30, 2013
1 parent 7c6c751 commit a80d79c
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 28 deletions.
3 changes: 2 additions & 1 deletion src/librust/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ fn cmd_test(args: &[~str]) -> ValidUsage {
let test_exec = Path(*filename).filestem().unwrap() + "test~";
invoke("rustc", &[~"--test", filename.to_owned(),
~"-o", test_exec.to_owned()], rustc::main_args);
let exit_code = run::process_status(~"./" + test_exec, []);
let args: &[&str] = [];
let exit_code = run::process_status(~"./" + test_exec, args);
Valid(exit_code)
}
_ => Invalid
Expand Down
54 changes: 30 additions & 24 deletions src/libstd/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use prelude::*;
use ptr;
use task;
use vec::ImmutableVector;
use str::Str;

/**
* A value representing a child process.
Expand Down Expand Up @@ -147,9 +148,8 @@ impl Process {
* * options - Options to configure the environment of the process,
* the working directory and the standard IO streams.
*/
pub fn new(prog: &str, args: &[~str],
options: ProcessOptions)
-> Process {
pub fn new<S: Str>(prog: &str, args: &[S], options: ProcessOptions)
-> Process {
#[fixed_stack_segment]; #[inline(never)];

let (in_pipe, in_fd) = match options.in_fd {
Expand Down Expand Up @@ -452,10 +452,10 @@ struct SpawnProcessResult {
}

#[cfg(windows)]
fn spawn_process_os(prog: &str, args: &[~str],
env: Option<~[(~str, ~str)]>,
dir: Option<&Path>,
in_fd: c_int, out_fd: c_int, err_fd: c_int) -> SpawnProcessResult {
fn spawn_process_os<S: Str>(prog: &str, args: &[S],
env: Option<~[(~str, ~str)]>,
dir: Option<&Path>,
in_fd: c_int, out_fd: c_int, err_fd: c_int) -> SpawnProcessResult {
#[fixed_stack_segment]; #[inline(never)];

use libc::types::os::arch::extra::{DWORD, HANDLE, STARTUPINFO};
Expand Down Expand Up @@ -584,12 +584,12 @@ fn zeroed_process_information() -> libc::types::os::arch::extra::PROCESS_INFORMA

// FIXME: this is only pub so it can be tested (see issue #4536)
#[cfg(windows)]
pub fn make_command_line(prog: &str, args: &[~str]) -> ~str {
pub fn make_command_line<S: Str>(prog: &str, args: &[S]) -> ~str {
let mut cmd = ~"";
append_arg(&mut cmd, prog);
for arg in args.iter() {
cmd.push_char(' ');
append_arg(&mut cmd, *arg);
append_arg(&mut cmd, arg.as_slice());
}
return cmd;

Expand Down Expand Up @@ -636,10 +636,10 @@ pub fn make_command_line(prog: &str, args: &[~str]) -> ~str {
}

#[cfg(unix)]
fn spawn_process_os(prog: &str, args: &[~str],
env: Option<~[(~str, ~str)]>,
dir: Option<&Path>,
in_fd: c_int, out_fd: c_int, err_fd: c_int) -> SpawnProcessResult {
fn spawn_process_os<S: Str>(prog: &str, args: &[S],
env: Option<~[(~str, ~str)]>,
dir: Option<&Path>,
in_fd: c_int, out_fd: c_int, err_fd: c_int) -> SpawnProcessResult {
#[fixed_stack_segment]; #[inline(never)];

use libc::funcs::posix88::unistd::{fork, dup2, close, chdir, execvp};
Expand Down Expand Up @@ -700,7 +700,7 @@ fn spawn_process_os(prog: &str, args: &[~str],
}

#[cfg(unix)]
fn with_argv<T>(prog: &str, args: &[~str], cb: &fn(**libc::c_char) -> T) -> T {
fn with_argv<T, S: Str>(prog: &str, args: &[S], cb: &fn(**libc::c_char) -> T) -> T {
use vec;

// We can't directly convert `str`s into `*char`s, as someone needs to hold
Expand All @@ -711,7 +711,7 @@ fn with_argv<T>(prog: &str, args: &[~str], cb: &fn(**libc::c_char) -> T) -> T {
tmps.push(prog.to_c_str());

for arg in args.iter() {
tmps.push(arg.to_c_str());
tmps.push(arg.as_slice().to_c_str());
}

// Next, convert each of the byte strings into a pointer. This is
Expand Down Expand Up @@ -817,7 +817,7 @@ fn free_handle(_handle: *()) {
*
* The process's exit code
*/
pub fn process_status(prog: &str, args: &[~str]) -> int {
pub fn process_status<S: Str>(prog: &str, args: &[S]) -> int {
let mut prog = Process::new(prog, args, ProcessOptions {
env: None,
dir: None,
Expand All @@ -840,7 +840,7 @@ pub fn process_status(prog: &str, args: &[~str]) -> int {
*
* The process's stdout/stderr output and exit code.
*/
pub fn process_output(prog: &str, args: &[~str]) -> ProcessOutput {
pub fn process_output<S: Str>(prog: &str, args: &[S]) -> ProcessOutput {
let mut prog = Process::new(prog, args, ProcessOptions::new());
prog.finish_with_output()
}
Expand Down Expand Up @@ -981,8 +981,9 @@ mod tests {
#[test]
#[cfg(not(target_os="android"))]
fn test_process_status() {
assert_eq!(run::process_status("false", []), 1);
assert_eq!(run::process_status("true", []), 0);
let args: &[&str] = [];
assert_eq!(run::process_status("false", args), 1);
assert_eq!(run::process_status("true", args), 0);
}
#[test]
#[cfg(target_os="android")]
Expand Down Expand Up @@ -1052,7 +1053,8 @@ mod tests {
let pipe_out = os::pipe();
let pipe_err = os::pipe();
let mut proc = run::Process::new("cat", [], run::ProcessOptions {
let args: &[&str] = [];
let mut proc = run::Process::new("cat", args, run::ProcessOptions {
dir: None,
env: None,
in_fd: Some(pipe_in.input),
Expand Down Expand Up @@ -1098,7 +1100,8 @@ mod tests {
#[test]
#[cfg(not(target_os="android"))]
fn test_finish_once() {
let mut prog = run::Process::new("false", [], run::ProcessOptions::new());
let args: &[&str] = [];
let mut prog = run::Process::new("false", args, run::ProcessOptions::new());
assert_eq!(prog.finish(), 1);
}
#[test]
Expand All @@ -1112,7 +1115,8 @@ mod tests {
#[test]
#[cfg(not(target_os="android"))]
fn test_finish_twice() {
let mut prog = run::Process::new("false", [], run::ProcessOptions::new());
let args: &[&str] = [];
let mut prog = run::Process::new("false", args, run::ProcessOptions::new());
assert_eq!(prog.finish(), 1);
assert_eq!(prog.finish(), 1);
}
Expand Down Expand Up @@ -1247,7 +1251,8 @@ mod tests {
#[cfg(unix,not(target_os="android"))]
fn run_pwd(dir: Option<&Path>) -> run::Process {
run::Process::new("pwd", [], run::ProcessOptions {
let args: &[&str] = [];
run::Process::new("pwd", args, run::ProcessOptions {
dir: dir,
.. run::ProcessOptions::new()
})
Expand Down Expand Up @@ -1302,7 +1307,8 @@ mod tests {
#[cfg(unix,not(target_os="android"))]
fn run_env(env: Option<~[(~str, ~str)]>) -> run::Process {
run::Process::new("env", [], run::ProcessOptions {
let args: &[&str] = [];
run::Process::new("env", args, run::ProcessOptions {
env: env,
.. run::ProcessOptions::new()
})
Expand Down
9 changes: 6 additions & 3 deletions src/test/run-pass/core-run-destroy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ use std::str;

#[test]
fn test_destroy_once() {
let mut p = run::Process::new("echo", [], run::ProcessOptions::new());
let args: &[&str] = [];
let mut p = run::Process::new("echo", args, run::ProcessOptions::new());
p.destroy(); // this shouldn't crash (and nor should the destructor)
}

#[test]
fn test_destroy_twice() {
let mut p = run::Process::new("echo", [], run::ProcessOptions::new());
let args: &[&str] = [];
let mut p = run::Process::new("echo", args, run::ProcessOptions::new());
p.destroy(); // this shouldnt crash...
p.destroy(); // ...and nor should this (and nor should the destructor)
}
Expand Down Expand Up @@ -74,7 +76,8 @@ fn test_destroy_actually_kills(force: bool) {
}

// this process will stay alive indefinitely trying to read from stdin
let mut p = run::Process::new(BLOCK_COMMAND, [], run::ProcessOptions::new());
let args: &[&str] = [];
let mut p = run::Process::new(BLOCK_COMMAND, args, run::ProcessOptions::new());

assert!(process_exists(p.get_id()));

Expand Down

4 comments on commit a80d79c

@bors
Copy link
Contributor

@bors bors commented on a80d79c Aug 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from erickt
at lilyball@a80d79c

@bors
Copy link
Contributor

@bors bors commented on a80d79c Aug 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging kballard/rust/process-new-args = a80d79c into auto

@bors
Copy link
Contributor

@bors bors commented on a80d79c Aug 30, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kballard/rust/process-new-args = a80d79c merged ok, testing candidate = a75d4cae

@bors
Copy link
Contributor

@bors bors commented on a80d79c Aug 30, 2013

Please sign in to comment.