Skip to content

Commit 503d50b

Browse files
authored
Rollup merge of #82464 - ehuss:unix-command-comment, r=kennytm
Update outdated comment in unix Command. The big comment in the `Command` struct has been incorrect for some time (at least since #46789 which removed `envp`). Rather than try to remove the allocations, this PR just updates the comment to reflect reality. There is an explanation for the reasoning at #31409 (comment), discussing the potential of being able to call `Command::exec` after `libc::fork`. That can still be done in the future, but I think for now it would be good to just correct the comment.
2 parents fe6cbbc + 476c6c2 commit 503d50b

File tree

2 files changed

+7
-17
lines changed

2 files changed

+7
-17
lines changed

library/std/src/sys/unix/ext/process.rs

+2
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,8 @@ impl CommandExt for process::Command {
172172
}
173173

174174
fn exec(&mut self) -> io::Error {
175+
// NOTE: This may *not* be safe to call after `libc::fork`, because it
176+
// may allocate. That may be worth fixing at some point in the future.
175177
self.as_inner_mut().exec(sys::process::Stdio::Inherit)
176178
}
177179

library/std/src/sys/unix/process/process_common.rs

+5-17
Original file line numberDiff line numberDiff line change
@@ -60,25 +60,13 @@ cfg_if::cfg_if! {
6060
////////////////////////////////////////////////////////////////////////////////
6161

6262
pub struct Command {
63-
// Currently we try hard to ensure that the call to `.exec()` doesn't
64-
// actually allocate any memory. While many platforms try to ensure that
65-
// memory allocation works after a fork in a multithreaded process, it's
66-
// been observed to be buggy and somewhat unreliable, so we do our best to
67-
// just not do it at all!
68-
//
69-
// Along those lines, the `argv` and `envp` raw pointers here are exactly
70-
// what's gonna get passed to `execvp`. The `argv` array starts with the
71-
// `program` and ends with a NULL, and the `envp` pointer, if present, is
72-
// also null-terminated.
73-
//
74-
// Right now we don't support removing arguments, so there's no much fancy
75-
// support there, but we support adding and removing environment variables,
76-
// so a side table is used to track where in the `envp` array each key is
77-
// located. Whenever we add a key we update it in place if it's already
78-
// present, and whenever we remove a key we update the locations of all
79-
// other keys.
8063
program: CString,
8164
args: Vec<CString>,
65+
/// Exactly what will be passed to `execvp`.
66+
///
67+
/// First element is a pointer to `program`, followed by pointers to
68+
/// `args`, followed by a `null`. Be careful when modifying `program` or
69+
/// `args` to properly update this as well.
8270
argv: Argv,
8371
env: CommandEnv,
8472

0 commit comments

Comments
 (0)