]> git.lizzy.rs Git - rust.git/commitdiff
Auto merge of #25641 - geofft:execve-const, r=alexcrichton
authorbors <bors@rust-lang.org>
Sun, 21 Jun 2015 17:45:01 +0000 (17:45 +0000)
committerbors <bors@rust-lang.org>
Sun, 21 Jun 2015 17:45:01 +0000 (17:45 +0000)
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.


Trivial merge