From 4e8e997d02625ea8bc91c0699bd358e971569333 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Sat, 7 Jun 2014 23:35:10 +0200 Subject: [PATCH 1/4] Add error handling to std::os::setenv. Error handling through std::io::IoResult. --- src/libstd/os.rs | 64 +++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 9a7e061c47282..a100ff3d37725 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -48,6 +48,7 @@ use str; use string::String; use sync::atomics::{AtomicInt, INIT_ATOMIC_INT, SeqCst}; use vec::Vec; +use io::{IoResult, IoError}; #[cfg(unix)] use c_str::ToCStr; @@ -380,13 +381,31 @@ pub fn getenv_as_bytes(n: &str) -> Option> { /// /// # Failure /// -/// Fails if `n` or `v` have any interior NULs. -pub fn setenv(n: &str, v: &str) { +/// Return Err if: +/// * `n` has length 0, or contain '=' character. +/// * There is no memory for adding to a new variable to the environment. +/// +/// # Example +/// +/// ```rust +/// let key = "KEY"; +/// let value = "VALUE"; +/// std::os::setenv(key.as_slice(), value); +/// match std::os::getenv(key) { +/// Some(ref val) => println!("{}: {}", key, val), +/// None => println!("{} is not defined in the environment.", key) +/// } +/// ``` +pub fn setenv(n: &str, v: &str) -> IoResult<()> { unsafe { with_env_lock(|| { n.with_c_str(|nbuf| { v.with_c_str(|vbuf| { - libc::funcs::posix01::unistd::setenv(nbuf, vbuf, 1); + if libc::funcs::posix01::unistd::setenv(nbuf, vbuf, 1) == 0 { + Ok(()) + } else { + Err(IoError::last_error()) + } }) }) }) @@ -397,13 +416,17 @@ pub fn setenv(n: &str, v: &str) { #[cfg(windows)] /// Sets the environment variable `n` to the value `v` for the currently running /// process -pub fn setenv(n: &str, v: &str) { +pub fn setenv(n: &str, v: &str) -> IoResult<()> { unsafe { with_env_lock(|| { use os::win32::as_utf16_p; as_utf16_p(n, |nbuf| { as_utf16_p(v, |vbuf| { - libc::SetEnvironmentVariableW(nbuf, vbuf); + if libc::SetEnvironmentVariableW(nbuf, vbuf) != 0 { + Ok(()) + } else { + Err(IoError::last_error()) + } }) }) }) @@ -1608,11 +1631,7 @@ mod tests { use os; use rand::Rng; use rand; - - #[test] - pub fn last_os_error() { - debug!("{}", os::last_os_error()); - } + use libc; fn make_rand_name() -> String { let mut rng = rand::task_rng(); @@ -1622,10 +1641,33 @@ mod tests { n } + #[test] + pub fn last_os_error() { + debug!("{}", os::last_os_error()); + } + + #[test] + #[cfg(unix)] + fn test_setenv_with_name_containing_equal() { + let n = format!("a={}", make_rand_name()); + let ret = setenv(n.as_slice(), "VALUE"); + assert!(ret.is_err()); + assert_eq!(ret.err().unwrap().kind, libc::EINVAL); + } + + #[test] + #[cfg(windows)] + fn test_setenv_with_name_containing_equal() { + let n = format!("a={}", make_rand_name()); + let ret = setenv(n.as_slice(), "VALUE"); + assert!(ret.is_err()); + assert_eq!(ret.err().unwrap().kind, libc::WSAEINVAL); + } + #[test] fn test_setenv() { let n = make_rand_name(); - setenv(n.as_slice(), "VALUE"); + assert!(setenv(n.as_slice(), "VALUE").is_ok()); assert_eq!(getenv(n.as_slice()), option::Some("VALUE".to_string())); } From f0bf49f5f5f2b7461218d85549451751ff4c9e9b Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Sat, 7 Jun 2014 23:42:48 +0200 Subject: [PATCH 2/4] Applying modif related to setenv. --- src/compiletest/compiletest.rs | 7 ++++++- src/libstd/unstable/dynamic_lib.rs | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/compiletest/compiletest.rs b/src/compiletest/compiletest.rs index 8fcad94ee1cfc..3f6d47a5fcc26 100644 --- a/src/compiletest/compiletest.rs +++ b/src/compiletest/compiletest.rs @@ -252,7 +252,12 @@ pub fn run_tests(config: &Config) { //arm-linux-androideabi debug-info test uses remote debugger //so, we test 1 task at once. // also trying to isolate problems with adb_run_wrapper.sh ilooping - os::setenv("RUST_TEST_TASKS","1"); + match os::setenv("RUST_TEST_TASKS","1") { + Ok(()) => {}, + Err(e) => { + fail!("`setenv` failed: {}", e); + } + } } let opts = test_opts(config); diff --git a/src/libstd/unstable/dynamic_lib.rs b/src/libstd/unstable/dynamic_lib.rs index c05cdc85cc53a..0c235191c2250 100644 --- a/src/libstd/unstable/dynamic_lib.rs +++ b/src/libstd/unstable/dynamic_lib.rs @@ -26,6 +26,7 @@ use os; use path::{Path,GenericPath}; use result::*; use slice::{Vector,ImmutableVector}; +use io::{IoResult}; use str; use string::String; use vec::Vec; @@ -78,12 +79,12 @@ impl DynamicLibrary { } /// Prepends a path to this process's search path for dynamic libraries - pub fn prepend_search_path(path: &Path) { + pub fn prepend_search_path(path: &Path) -> IoResult<()> { let mut search_path = DynamicLibrary::search_path(); search_path.insert(0, path.clone()); let newval = DynamicLibrary::create_path(search_path.as_slice()); os::setenv(DynamicLibrary::envvar(), - str::from_utf8(newval.as_slice()).unwrap()); + str::from_utf8(newval.as_slice()).unwrap()) } /// From a slice of paths, create a new vector which is suitable to be an From dea2a3ffcfdfd3f052e433160a331bfce222a6ea Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Sat, 7 Jun 2014 23:43:33 +0200 Subject: [PATCH 3/4] fixup! Dirty hack I need some help on it. --- src/librustc/metadata/filesearch.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc/metadata/filesearch.rs b/src/librustc/metadata/filesearch.rs index 9033b83d47420..c0acc73be55d5 100644 --- a/src/librustc/metadata/filesearch.rs +++ b/src/librustc/metadata/filesearch.rs @@ -134,6 +134,9 @@ impl<'a> FileSearch<'a> { } } + // FIXME: # It's a dirty hack. + // A better error handling is needed here. + #[allow(unused_must_use)] pub fn add_dylib_search_paths(&self) { self.for_each_lib_search_path(|lib_search_path| { DynamicLibrary::prepend_search_path(lib_search_path); From cd20277b6ab1fa511a2b68959ab732f9ecd9e6f3 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Sun, 8 Jun 2014 15:45:06 +0200 Subject: [PATCH 4/4] fixup! Various fixes. --- src/libstd/os.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libstd/os.rs b/src/libstd/os.rs index a100ff3d37725..5f12ee6163d3d 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -390,8 +390,11 @@ pub fn getenv_as_bytes(n: &str) -> Option> { /// ```rust /// let key = "KEY"; /// let value = "VALUE"; -/// std::os::setenv(key.as_slice(), value); -/// match std::os::getenv(key) { +/// match std::os::setenv(key.as_slice(), value.as_slice()) { +/// Ok(()) => {}, +/// Err(e) => fail!("setenv failed: {}", e) +/// } +/// match std::os::getenv(key.as_slice()) { /// Some(ref val) => println!("{}: {}", key, val), /// None => println!("{} is not defined in the environment.", key) /// } @@ -1784,13 +1787,13 @@ mod tests { let oldhome = getenv("HOME"); setenv("HOME", "/home/MountainView"); - assert!(os::homedir() == Some(Path::new("/home/MountainView"))); + assert_eq!(os::homedir(), Some(Path::new("/home/MountainView"))); setenv("HOME", ""); assert!(os::homedir().is_none()); for s in oldhome.iter() { - setenv("HOME", s.as_slice()) + setenv("HOME", s.as_slice()); } } @@ -1819,10 +1822,10 @@ mod tests { assert!(os::homedir() == Some(Path::new("/home/MountainView"))); for s in oldhome.iter() { - setenv("HOME", s.as_slice()) + setenv("HOME", s.as_slice()); } for s in olduserprofile.iter() { - setenv("USERPROFILE", s.as_slice()) + setenv("USERPROFILE", s.as_slice()); } }