Skip to content

Commit

Permalink
Auto merge of rust-lang#25641 - geofft:execve-const, r=alexcrichton
Browse files Browse the repository at this point in the history
The `execv` family of functions and `getopt` are prototyped somewhat strangely in C: they take a `char *const argv[]` (and `envp`, for `execve`), an immutable array of mutable C strings -- in other words, a `char *const *argv` or `argv: *const *mut c_char`. The current Rust binding uses `*mut *const c_char`, which is backwards (a mutable array of constant C strings).

That said, these functions do not actually modify their arguments. Once upon a time, C didn't have `const`, and to this day, string literals in C have type `char *` (`*mut c_char`). So an array of string literals has type `char * []`, equivalent to `char **` in a function parameter (Rust `*mut *mut c_char`). C allows an implicit cast from `T **` to `T * const *` (`*const *mut T`) but not to `const T * const *` (`*const *const T`). Therefore, prototyping `execv` as taking `const char * const argv[]` would have broken existing code (by requiring an explicit cast), despite being more correct. So, even though these functions don't need mutable data, they're prototyped as if they do.

While it's theoretically possible that an implementation could choose to use its freedom to modify the mutable data, such an implementation would break the innumerable users of `execv`-family functions that call them with string literals. Such an implementation would also break `std::process`, which currently works around this with an unsafe `as *mut _` cast, and assumes that `execvp` secretly does not modify its argument. Furthermore, there's nothing useful to be gained by being able to write to the strings in `argv` themselves but not being able to write to the array containing those strings (you can't reorder arguments, add arguments, increase the length of any parameter, etc.).

So, despite the C prototype with its legacy C problems, it's simpler for everyone for Rust to consider these functions as taking `*const *const c_char`, and it's also safe to do so. Rust does not need to expose the mistakes of C here. This patch makes that change, and drops the unsafe cast in `std::process` since it's now unnecessary.
  • Loading branch information
bors committed Jun 21, 2015
2 parents a38e758 + 058a0f0 commit dedd430
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 17 deletions.
34 changes: 18 additions & 16 deletions src/liblibc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5493,17 +5493,17 @@ pub mod funcs {
pub fn dup2(src: c_int, dst: c_int) -> c_int;
#[link_name = "_execv"]
pub fn execv(prog: *const c_char,
argv: *mut *const c_char) -> intptr_t;
argv: *const *const c_char) -> intptr_t;
#[link_name = "_execve"]
pub fn execve(prog: *const c_char, argv: *mut *const c_char,
envp: *mut *const c_char)
pub fn execve(prog: *const c_char, argv: *const *const c_char,
envp: *const *const c_char)
-> c_int;
#[link_name = "_execvp"]
pub fn execvp(c: *const c_char,
argv: *mut *const c_char) -> c_int;
argv: *const *const c_char) -> c_int;
#[link_name = "_execvpe"]
pub fn execvpe(c: *const c_char, argv: *mut *const c_char,
envp: *mut *const c_char) -> c_int;
pub fn execvpe(c: *const c_char, argv: *const *const c_char,
envp: *const *const c_char) -> c_int;
#[link_name = "_getcwd"]
pub fn getcwd(buf: *mut c_char, size: size_t) -> *mut c_char;
#[link_name = "_getpid"]
Expand Down Expand Up @@ -5687,12 +5687,12 @@ pub mod funcs {
pub fn dup(fd: c_int) -> c_int;
pub fn dup2(src: c_int, dst: c_int) -> c_int;
pub fn execv(prog: *const c_char,
argv: *mut *const c_char) -> c_int;
pub fn execve(prog: *const c_char, argv: *mut *const c_char,
envp: *mut *const c_char)
argv: *const *const c_char) -> c_int;
pub fn execve(prog: *const c_char, argv: *const *const c_char,
envp: *const *const c_char)
-> c_int;
pub fn execvp(c: *const c_char,
argv: *mut *const c_char) -> c_int;
argv: *const *const c_char) -> c_int;
pub fn fork() -> pid_t;
pub fn fpathconf(filedes: c_int, name: c_int) -> c_long;
pub fn getcwd(buf: *mut c_char, size: size_t) -> *mut c_char;
Expand All @@ -5702,7 +5702,9 @@ pub mod funcs {
pub fn getgroups(ngroups_max: c_int, groups: *mut gid_t)
-> c_int;
pub fn getlogin() -> *mut c_char;
pub fn getopt(argc: c_int, argv: *mut *const c_char,
// GNU getopt(3) modifies its arguments despite the
// char * const [] prototype; see the manpage.
pub fn getopt(argc: c_int, argv: *mut *mut c_char,
optstr: *const c_char) -> c_int;
pub fn getpgrp() -> pid_t;
pub fn getpid() -> pid_t;
Expand Down Expand Up @@ -5752,19 +5754,19 @@ pub mod funcs {
pub fn dup(fd: c_int) -> c_int;
pub fn dup2(src: c_int, dst: c_int) -> c_int;
pub fn execv(prog: *const c_char,
argv: *mut *const c_char) -> c_int;
pub fn execve(prog: *const c_char, argv: *mut *const c_char,
envp: *mut *const c_char)
argv: *const *const c_char) -> c_int;
pub fn execve(prog: *const c_char, argv: *const *const c_char,
envp: *const *const c_char)
-> c_int;
pub fn execvp(c: *const c_char,
argv: *mut *const c_char) -> c_int;
argv: *const *const c_char) -> c_int;
pub fn fork() -> pid_t;
pub fn getcwd(buf: *mut c_char, size: size_t) -> *mut c_char;
pub fn getegid() -> gid_t;
pub fn geteuid() -> uid_t;
pub fn getgid() -> gid_t;
pub fn getlogin() -> *mut c_char;
pub fn getopt(argc: c_int, argv: *mut *const c_char,
pub fn getopt(argc: c_int, argv: *const *const c_char,
optstr: *const c_char) -> c_int;
pub fn getuid() -> uid_t;
pub fn getsid(pid: pid_t) -> pid_t;
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/unix/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl Process {
if !envp.is_null() {
*sys::os::environ() = envp as *const _;
}
let _ = libc::execvp(*argv, argv as *mut _);
let _ = libc::execvp(*argv, argv);
fail(&mut output)
}

Expand Down

0 comments on commit dedd430

Please sign in to comment.