Add unstable env::set and env::remove

Author: KamilaBorowska

library/std/src/env.rs

@@ -298,19 +298,48 @@ impl Error for VarError {
     }
 }
 
+#[unstable(feature = "unsafe_env", issue = "none")]
 /// Sets the environment variable `key` to the value `value` for the currently running
 /// process.
 ///
-/// Note that while concurrent access to environment variables is safe in Rust,
-/// some platforms only expose inherently unsafe non-threadsafe APIs for
-/// inspecting the environment. As a result, extra care needs to be taken when
-/// auditing calls to unsafe external FFI functions to ensure that any external
-/// environment accesses are properly synchronized with accesses in Rust.
+/// # Safety
 ///
-/// Discussion of this unsafety on Unix may be found in:
+/// This function must not be called in the presence of concurrent threads that
+/// may simultaneously read or write the environment. Also, it's not allowed
+/// to use this function in functions marked with `#[test]` attribute because
+/// test harness uses threads internally.
 ///
-///  - [Austin Group Bugzilla](https://austingroupbugs.net/view.php?id=188)
-///  - [GNU C library Bugzilla](https://sourceware.org/bugzilla/show_bug.cgi?id=15607#c2)
+/// # Panics
+///
+/// This function may panic if `key` is empty, contains an ASCII equals sign `'='`
+/// or the NUL character `'\0'`, or when `value` contains the NUL character.
+///
+/// # Examples
+///
+/// ```
+/// #![feature(unsafe_env)]
+///
+/// use std::env;
+///
+/// let key = "KEY";
+/// unsafe {
+///     env::set(key, "VALUE");
+/// }
+/// assert_eq!(env::var(key), Ok("VALUE".to_string()));
+/// ```
+pub unsafe fn set<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) {
+    _set_var(key.as_ref(), value.as_ref(), |k, v| os_imp::setenv(k, v))
+}
+
+/// Sets the environment variable `key` to the value `value` for the currently running
+/// process.
+///
+/// # Safety
+///
+/// This function must not be called in the presence of concurrent threads that
+/// may simultaneously read or write the environment. Also, it's not allowed
+/// to use this function in functions marked with `#[test]` attribute because
+/// test harness uses threads internally.
 ///
 /// # Panics
 ///
@@ -328,27 +357,59 @@ impl Error for VarError {
 /// ```
 #[stable(feature = "env", since = "1.0.0")]
 pub fn set_var<K: AsRef<OsStr>, V: AsRef<OsStr>>(key: K, value: V) {
-    _set_var(key.as_ref(), value.as_ref())
+    _set_var(key.as_ref(), value.as_ref(), |k, v| unsafe { os_imp::setenv_locking(k, v) })
 }
 
-fn _set_var(key: &OsStr, value: &OsStr) {
-    os_imp::setenv(key, value).unwrap_or_else(|e| {
+fn _set_var<F: Fn(&OsStr, &OsStr) -> io::Result<()>>(key: &OsStr, value: &OsStr, f: F) {
+    f(key, value).unwrap_or_else(|e| {
         panic!("failed to set environment variable `{:?}` to `{:?}`: {}", key, value, e)
     })
 }
 
+#[unstable(feature = "unsafe_env", issue = "none")]
 /// Removes an environment variable from the environment of the currently running process.
 ///
-/// Note that while concurrent access to environment variables is safe in Rust,
-/// some platforms only expose inherently unsafe non-threadsafe APIs for
-/// inspecting the environment. As a result extra care needs to be taken when
-/// auditing calls to unsafe external FFI functions to ensure that any external
-/// environment accesses are properly synchronized with accesses in Rust.
+/// # Safety
+///
+/// This function must not be called in the presence of concurrent threads that
+/// may simultaneously read or write the environment. Also, it's not allowed
+/// to use this function in functions marked with `#[test]` attribute because
+/// test harness uses threads internally.
+///
+/// # Panics
+///
+/// This function may panic if `key` is empty, contains an ASCII equals sign
+/// `'='` or the NUL character `'\0'`, or when the value contains the NUL
+/// character.
+///
+/// # Examples
+///
+/// ```
+/// #![feature(unsafe_env)]
+///
+/// use std::env;
+///
+/// let key = "KEY";
+/// env::set_var(key, "VALUE");
+/// assert_eq!(env::var(key), Ok("VALUE".to_string()));
+///
+/// unsafe {
+///     env::remove(key);
+/// }
+/// assert!(env::var(key).is_err());
+/// ```
+pub unsafe fn remove<K: AsRef<OsStr>>(key: K) {
+    _remove_var(key.as_ref(), |n| os_imp::unsetenv(n))
+}
+
+/// Removes an environment variable from the environment of the currently running process.
 ///
-/// Discussion of this unsafety on Unix may be found in:
+/// # Safety
 ///
-///  - [Austin Group Bugzilla](https://austingroupbugs.net/view.php?id=188)
-///  - [GNU C library Bugzilla](https://sourceware.org/bugzilla/show_bug.cgi?id=15607#c2)
+/// This function must not be called in the presence of concurrent threads that
+/// may simultaneously read or write the environment. Also, it's not allowed
+/// to use this function in functions marked with `#[test]` attribute because
+/// test harness uses threads internally.
 ///
 /// # Panics
 ///
@@ -370,12 +431,11 @@ fn _set_var(key: &OsStr, value: &OsStr) {
 /// ```
 #[stable(feature = "env", since = "1.0.0")]
 pub fn remove_var<K: AsRef<OsStr>>(key: K) {
-    _remove_var(key.as_ref())
+    _remove_var(key.as_ref(), |n| unsafe { os_imp::unsetenv_locking(n) })
 }
 
-fn _remove_var(key: &OsStr) {
-    os_imp::unsetenv(key)
-        .unwrap_or_else(|e| panic!("failed to remove environment variable `{:?}`: {}", key, e))
+fn _remove_var<F: Fn(&OsStr) -> io::Result<()>>(key: &OsStr, f: F) {
+    f(key).unwrap_or_else(|e| panic!("failed to remove environment variable `{:?}`: {}", key, e))
 }
 
 /// An iterator that splits an environment variable into paths according to

library/std/src/sys/hermit/os.rs

@@ -144,18 +144,22 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
     unsafe { ENV.as_ref().unwrap().lock().unwrap().get_mut(k).cloned() }
 }
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
-    unsafe {
-        let (k, v) = (k.to_owned(), v.to_owned());
-        ENV.as_ref().unwrap().lock().unwrap().insert(k, v);
-    }
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    setenv_locking(k, v)
+}
+
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    let (k, v) = (k.to_owned(), v.to_owned());
+    ENV.as_ref().unwrap().lock().unwrap().insert(k, v);
     Ok(())
 }
 
-pub fn unsetenv(k: &OsStr) -> io::Result<()> {
-    unsafe {
-        ENV.as_ref().unwrap().lock().unwrap().remove(k);
-    }
+pub unsafe fn unsetenv(k: &OsStr) -> io::Result<()> {
+    unsetenv_locking(k)
+}
+
+pub unsafe fn unsetenv_locking(k: &OsStr) -> io::Result<()> {
+    ENV.as_ref().unwrap().lock().unwrap().remove(k);
     Ok(())
 }
 

library/std/src/sys/sgx/os.rs

@@ -110,13 +110,21 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
     get_env_store().and_then(|s| s.lock().unwrap().get(k).cloned())
 }
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    setenv_locking(k, v)
+}
+
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let (k, v) = (k.to_owned(), v.to_owned());
     create_env_store().lock().unwrap().insert(k, v);
     Ok(())
 }
 
-pub fn unsetenv(k: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv(k: &OsStr) -> io::Result<()> {
+    unsetenv_locking(k)
+}
+
+pub unsafe fn unsetenv_locking(k: &OsStr) -> io::Result<()> {
     if let Some(env) = get_env_store() {
         env.lock().unwrap().remove(k);
     }

library/std/src/sys/solid/os.rs

@@ -151,23 +151,28 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
     }
 }
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let k = CString::new(k.as_bytes())?;
     let v = CString::new(v.as_bytes())?;
+    cvt_env(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
+}
 
-    unsafe {
-        let _guard = ENV_LOCK.write();
-        cvt_env(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
-    }
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    let k = CString::new(k.as_bytes())?;
+    let v = CString::new(v.as_bytes())?;
+    let _guard = ENV_LOCK.write();
+    cvt_env(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
 }
 
-pub fn unsetenv(n: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> {
     let nbuf = CString::new(n.as_bytes())?;
+    cvt_env(libc::unsetenv(nbuf.as_ptr())).map(drop)
+}
 
-    unsafe {
-        let _guard = ENV_LOCK.write();
-        cvt_env(libc::unsetenv(nbuf.as_ptr())).map(drop)
-    }
+pub unsafe fn unsetenv_locking(n: &OsStr) -> io::Result<()> {
+    let nbuf = CString::new(n.as_bytes())?;
+    let _guard = ENV_LOCK.write();
+    cvt_env(libc::unsetenv(nbuf.as_ptr())).map(drop)
 }
 
 /// In kmclib, `setenv` and `unsetenv` don't always set `errno`, so this

library/std/src/sys/unix/os.rs

@@ -541,23 +541,28 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
     }
 }
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let k = CString::new(k.as_bytes())?;
     let v = CString::new(v.as_bytes())?;
+    cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
+}
 
-    unsafe {
-        let _guard = ENV_LOCK.write();
-        cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
-    }
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    let k = CString::new(k.as_bytes())?;
+    let v = CString::new(v.as_bytes())?;
+    let _guard = ENV_LOCK.write();
+    cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
 }
 
-pub fn unsetenv(n: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> {
     let nbuf = CString::new(n.as_bytes())?;
+    cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
+}
 
-    unsafe {
-        let _guard = ENV_LOCK.write();
-        cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
-    }
+pub unsafe fn unsetenv_locking(n: &OsStr) -> io::Result<()> {
+    let nbuf = CString::new(n.as_bytes())?;
+    let _guard = ENV_LOCK.write();
+    cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
 }
 
 #[cfg(not(target_os = "espidf"))]

library/std/src/sys/unsupported/os.rs

@@ -80,11 +80,19 @@ pub fn getenv(_: &OsStr) -> Option<OsString> {
     None
 }
 
-pub fn setenv(_: &OsStr, _: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    setenv_locking(k, v)
+}
+
+pub unsafe fn setenv_locking(_: &OsStr, _: &OsStr) -> io::Result<()> {
     Err(io::Error::new_const(io::ErrorKind::Unsupported, &"cannot set env vars on this platform"))
 }
 
-pub fn unsetenv(_: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv(k: &OsStr) -> io::Result<()> {
+    unsetenv_locking(k)
+}
+
+pub unsafe fn unsetenv_locking(_: &OsStr) -> io::Result<()> {
     Err(io::Error::new_const(io::ErrorKind::Unsupported, &"cannot unset env vars on this platform"))
 }
 

library/std/src/sys/wasi/os.rs

@@ -187,18 +187,25 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
         }
     }
 }
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    let k = CString::new(k.as_bytes())?;
+    let v = CString::new(v.as_bytes())?;
+    cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
+}
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let k = CString::new(k.as_bytes())?;
     let v = CString::new(v.as_bytes())?;
+    let _guard = env_lock();
+    cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
+}
 
-    unsafe {
-        let _guard = env_lock();
-        cvt(libc::setenv(k.as_ptr(), v.as_ptr(), 1)).map(drop)
-    }
+pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> {
+    let nbuf = CString::new(n.as_bytes())?;
+    cvt(libc::unsetenv(nbuf.as_ptr())).map(drop)
 }
 
-pub fn unsetenv(n: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv_locking(n: &OsStr) -> io::Result<()> {
     let nbuf = CString::new(n.as_bytes())?;
 
     unsafe {

library/std/src/sys/windows/os.rs

@@ -262,16 +262,24 @@ pub fn getenv(k: &OsStr) -> Option<OsString> {
     .ok()
 }
 
-pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+pub unsafe fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {
+    setenv_locking(k, v)
+}
+
+pub unsafe fn setenv_locking(k: &OsStr, v: &OsStr) -> io::Result<()> {
     let k = to_u16s(k)?;
     let v = to_u16s(v)?;
 
-    cvt(unsafe { c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr()) }).map(drop)
+    cvt(c::SetEnvironmentVariableW(k.as_ptr(), v.as_ptr())).map(drop)
+}
+
+pub unsafe fn unsetenv(n: &OsStr) -> io::Result<()> {
+    unsetenv_locking(n)
 }
 
-pub fn unsetenv(n: &OsStr) -> io::Result<()> {
+pub unsafe fn unsetenv_locking(n: &OsStr) -> io::Result<()> {
     let v = to_u16s(n)?;
-    cvt(unsafe { c::SetEnvironmentVariableW(v.as_ptr(), ptr::null()) }).map(drop)
+    cvt(c::SetEnvironmentVariableW(v.as_ptr(), ptr::null())).map(drop)
 }
 
 pub fn temp_dir() -> PathBuf {