-
Notifications
You must be signed in to change notification settings - Fork 674
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Provide clearenv() #1185
Provide clearenv() #1185
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
use {Error, Result}; | ||
|
||
/// Clear the environment of all name-value pairs. | ||
/// | ||
/// On platforms where libc provides `clearenv()`, it will be used. libc's | ||
/// `clearenv()` is documented to return an error code but not set errno; if the | ||
/// return value indicates a failure, this function will return | ||
/// `Error::UnsupportedOperation`. | ||
/// | ||
/// On platforms where libc does not provide `clearenv()`, a fallback | ||
/// implementation will be used that iterates over all environment variables and | ||
/// removes them one-by-one. | ||
/// | ||
/// # Safety | ||
/// | ||
/// This function is not threadsafe and can cause undefined behavior in | ||
/// combination with `std::env` or other program components that access the | ||
/// environment. See, for example, the discussion on `std::env::remove_var`; this | ||
/// function is a case of an "inherently unsafe non-threadsafe API" dealing with | ||
/// the environment. | ||
/// | ||
/// The caller must ensure no other threads access the process environment while | ||
/// this function executes and that no raw pointers to an element of libc's | ||
/// `environ` is currently held. The latter is not an issue if the only other | ||
/// environment access in the program is via `std::env`, but the requirement on | ||
/// thread safety must still be upheld. | ||
pub unsafe fn clearenv() -> Result<()> { | ||
let ret; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As a matter of style, I suggest: let ret = cfg_if! {
if something {
libc::clearenv()
} else {
0
}
}; There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried that first, but it looks like that's not allowed with
|
||
cfg_if! { | ||
if #[cfg(any(target_os = "fuchsia", | ||
target_os = "wasi", | ||
target_env = "wasi", | ||
target_env = "uclibc", | ||
target_os = "linux", | ||
target_os = "android", | ||
target_os = "emscripten"))] { | ||
ret = libc::clearenv(); | ||
} else { | ||
use std::env; | ||
for (name, _) in env::vars_os() { | ||
env::remove_var(name); | ||
} | ||
ret = 0; | ||
} | ||
} | ||
|
||
if ret == 0 { | ||
Ok(()) | ||
} else { | ||
Err(Error::UnsupportedOperation) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
extern crate nix; | ||
|
||
use std::env; | ||
|
||
#[test] | ||
fn clearenv() { | ||
env::set_var("FOO", "BAR"); | ||
unsafe { nix::env::clearenv() }.unwrap(); | ||
assert_eq!(env::var("FOO").unwrap_err(), env::VarError::NotPresent); | ||
assert_eq!(env::vars().count(), 0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you actually verify that 0.1.10 is the minimum version that works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I mentioned before that the error is still there on 0.1.9. I didn't try every version in between 0.1.2 and 0.1.9; it seems unlikely any of those work work if the same error is in both 0.1.2 and 0.1.9, but I guess it's not impossible. Is there an impact to jumping to 0.1.10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, given that 0.1.10 comes immediately after 0.1.9, I'd say that confirms it. Too bad that the cfg-if crate doesn't maintain a CHANGELOG.