From 6f422c4c05f4d108ba6429a174aa0c2ef3b183fa Mon Sep 17 00:00:00 2001
From: Barosl Lee <vcs@barosl.com>
Date: Tue, 11 Nov 2014 14:38:20 +0900
Subject: [PATCH 1/3] Make os::getcwd() return IoResult<Path>
os::getcwd() panics if the current directory is not available. According
to getcwd(3), there are three cases:
- EACCES: Permission denied.
- ENOENT: The current working directory has been removed.
- ERANGE: The buffer size is less than the actual absolute path.
This commit makes os::getcwd() return IoResult<Path>, not just Path,
preventing it from panicking.
As os::make_absolute() depends on os::getcwd(), it is also modified to
return IoResult<Path>.
Fixes #16946.
[breaking-change]
---
src/librustc/metadata/filesearch.rs | 2 +-
src/librustc/plugin/load.rs | 2 +-
src/librustc/session/mod.rs | 4 +-
src/librustc_back/archive.rs | 4 +-
src/librustc_back/fs.rs | 2 +-
src/librustc_back/rpath.rs | 6 +-
src/libstd/io/process.rs | 4 +-
src/libstd/io/tempfile.rs | 3 +-
src/libstd/io/test.rs | 2 +-
src/libstd/os.rs | 64 +++++++++++--------
.../process-spawn-with-unicode-params.rs | 2 +-
src/test/run-pass/tempfile.rs | 4 +-
12 files changed, 56 insertions(+), 43 deletions(-)
diff --git a/src/librustc/metadata/filesearch.rs b/src/librustc/metadata/filesearch.rs
index f7d666e48150d..fee289df3e4e4 100644
--- a/src/librustc/metadata/filesearch.rs
+++ b/src/librustc/metadata/filesearch.rs
@@ -219,7 +219,7 @@ pub fn rust_path() -> Vec<Path> {
}
None => Vec::new()
};
- let mut cwd = os::getcwd();
+ let mut cwd = os::getcwd().unwrap();
// now add in default entries
let cwd_dot_rust = cwd.join(".rust");
if !env_rust_path.contains(&cwd_dot_rust) {
diff --git a/src/librustc/plugin/load.rs b/src/librustc/plugin/load.rs
index 1a270d0b1b7a6..5c2fe0854ee77 100644
--- a/src/librustc/plugin/load.rs
+++ b/src/librustc/plugin/load.rs
@@ -134,7 +134,7 @@ impl<'a> PluginLoader<'a> {
// Dynamically link a registrar function into the compiler process.
fn dylink_registrar(&mut self, vi: &ast::ViewItem, path: Path, symbol: String) {
// Make sure the path contains a / or the linker will search for it.
- let path = os::make_absolute(&path);
+ let path = os::make_absolute(&path).unwrap();
let lib = match DynamicLibrary::open(Some(&path)) {
Ok(lib) => lib,
diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
index d723db7706fac..72a9f23aa1f94 100644
--- a/src/librustc/session/mod.rs
+++ b/src/librustc/session/mod.rs
@@ -231,7 +231,7 @@ pub fn build_session_(sopts: config::Options,
if path.is_absolute() {
path.clone()
} else {
- os::getcwd().join(&path)
+ os::getcwd().unwrap().join(&path)
}
);
@@ -246,7 +246,7 @@ pub fn build_session_(sopts: config::Options,
plugin_registrar_fn: Cell::new(None),
default_sysroot: default_sysroot,
local_crate_source_file: local_crate_source_file,
- working_dir: os::getcwd(),
+ working_dir: os::getcwd().unwrap(),
lint_store: RefCell::new(lint::LintStore::new()),
lints: RefCell::new(NodeMap::new()),
crate_types: RefCell::new(Vec::new()),
diff --git a/src/librustc_back/archive.rs b/src/librustc_back/archive.rs
index 369cef43622c7..a88bcafaa64b3 100644
--- a/src/librustc_back/archive.rs
+++ b/src/librustc_back/archive.rs
@@ -229,7 +229,7 @@ impl<'a> ArchiveBuilder<'a> {
pub fn build(self) -> Archive<'a> {
// Get an absolute path to the destination, so `ar` will work even
// though we run it from `self.work_dir`.
- let abs_dst = os::getcwd().join(&self.archive.dst);
+ let abs_dst = os::getcwd().unwrap().join(&self.archive.dst);
assert!(!abs_dst.is_relative());
let mut args = vec![&abs_dst];
let mut total_len = abs_dst.as_vec().len();
@@ -286,7 +286,7 @@ impl<'a> ArchiveBuilder<'a> {
// First, extract the contents of the archive to a temporary directory.
// We don't unpack directly into `self.work_dir` due to the possibility
// of filename collisions.
- let archive = os::make_absolute(archive);
+ let archive = os::make_absolute(archive).unwrap();
run_ar(self.archive.handler, &self.archive.maybe_ar_prog,
"x", Some(loc.path()), &[&archive]);
diff --git a/src/librustc_back/fs.rs b/src/librustc_back/fs.rs
index a062c68d998a9..d9bf804403955 100644
--- a/src/librustc_back/fs.rs
+++ b/src/librustc_back/fs.rs
@@ -16,7 +16,7 @@ use std::os;
/// returned path does not contain any symlinks in its hierarchy.
pub fn realpath(original: &Path) -> io::IoResult<Path> {
static MAX_LINKS_FOLLOWED: uint = 256;
- let original = os::make_absolute(original);
+ let original = os::make_absolute(original).unwrap();
// Right now lstat on windows doesn't work quite well
if cfg!(windows) {
diff --git a/src/librustc_back/rpath.rs b/src/librustc_back/rpath.rs
index 974d8f889c95e..26cc859434f2c 100644
--- a/src/librustc_back/rpath.rs
+++ b/src/librustc_back/rpath.rs
@@ -102,9 +102,9 @@ fn get_rpath_relative_to_output(config: &mut RPathConfig,
"$ORIGIN"
};
- let mut lib = (config.realpath)(&os::make_absolute(lib)).unwrap();
+ let mut lib = (config.realpath)(&os::make_absolute(lib).unwrap()).unwrap();
lib.pop();
- let mut output = (config.realpath)(&os::make_absolute(&config.out_filename)).unwrap();
+ let mut output = (config.realpath)(&os::make_absolute(&config.out_filename).unwrap()).unwrap();
output.pop();
let relative = lib.path_relative_from(&output);
let relative = relative.expect("could not create rpath relative to output");
@@ -116,7 +116,7 @@ fn get_rpath_relative_to_output(config: &mut RPathConfig,
fn get_install_prefix_rpath(config: RPathConfig) -> String {
let path = (config.get_install_prefix_lib_path)();
- let path = os::make_absolute(&path);
+ let path = os::make_absolute(&path).unwrap();
// FIXME (#9639): This needs to handle non-utf8 paths
path.as_str().expect("non-utf8 component in rpath").to_string()
}
diff --git a/src/libstd/io/process.rs b/src/libstd/io/process.rs
index 16e568f30f2ca..592ec0681a96a 100644
--- a/src/libstd/io/process.rs
+++ b/src/libstd/io/process.rs
@@ -974,7 +974,7 @@ mod tests {
let prog = pwd_cmd().spawn().unwrap();
let output = String::from_utf8(prog.wait_with_output().unwrap().output).unwrap();
- let parent_dir = os::getcwd();
+ let parent_dir = os::getcwd().unwrap();
let child_dir = Path::new(output.as_slice().trim());
let parent_stat = parent_dir.stat().unwrap();
@@ -989,7 +989,7 @@ mod tests {
use os;
// test changing to the parent of os::getcwd() because we know
// the path exists (and os::getcwd() is not expected to be root)
- let parent_dir = os::getcwd().dir_path();
+ let parent_dir = os::getcwd().unwrap().dir_path();
let prog = pwd_cmd().cwd(&parent_dir).spawn().unwrap();
let output = String::from_utf8(prog.wait_with_output().unwrap().output).unwrap();
diff --git a/src/libstd/io/tempfile.rs b/src/libstd/io/tempfile.rs
index e9d6ef2e34130..a232231733d8b 100644
--- a/src/libstd/io/tempfile.rs
+++ b/src/libstd/io/tempfile.rs
@@ -35,7 +35,8 @@ impl TempDir {
/// If no directory can be created, `Err` is returned.
pub fn new_in(tmpdir: &Path, suffix: &str) -> IoResult<TempDir> {
if !tmpdir.is_absolute() {
- return TempDir::new_in(&os::make_absolute(tmpdir), suffix);
+ let abs_tmpdir = try!(os::make_absolute(tmpdir));
+ return TempDir::new_in(&abs_tmpdir, suffix);
}
static CNT: atomic::AtomicUint = atomic::INIT_ATOMIC_UINT;
diff --git a/src/libstd/io/test.rs b/src/libstd/io/test.rs
index 6571dc41585bb..a153ead2a3843 100644
--- a/src/libstd/io/test.rs
+++ b/src/libstd/io/test.rs
@@ -75,7 +75,7 @@ fn base_port() -> u16 {
];
// FIXME (#9639): This needs to handle non-utf8 paths
- let path = os::getcwd();
+ let path = os::getcwd().unwrap();
let path_s = path.as_str().unwrap();
let mut final_base = base;
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index f7afce9f4099f..971138c06fb67 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -38,7 +38,7 @@ pub use self::MapError::*;
use clone::Clone;
use error::{FromError, Error};
use fmt;
-use io::IoResult;
+use io::{IoResult, IoError};
use iter::Iterator;
use libc::{c_void, c_int};
use libc;
@@ -76,15 +76,16 @@ pub fn num_cpus() -> uint {
pub const TMPBUF_SZ : uint = 1000u;
const BUF_BYTES : uint = 2048u;
-/// Returns the current working directory as a Path.
+/// Returns the current working directory as a `Path`.
///
-/// # Failure
+/// # Errors
///
-/// Fails if the current working directory value is invalid:
+/// Returns an `Err` if the current working directory value is invalid.
/// Possible cases:
///
/// * Current directory does not exist.
/// * There are insufficient permissions to access the current directory.
+/// * The internal buffer is not large enough to hold the path.
///
/// # Example
///
@@ -92,32 +93,34 @@ const BUF_BYTES : uint = 2048u;
/// use std::os;
///
/// // We assume that we are in a valid directory like "/home".
-/// let current_working_directory = os::getcwd();
+/// let current_working_directory = os::getcwd().unwrap();
/// println!("The current directory is {}", current_working_directory.display());
/// // /home
/// ```
#[cfg(unix)]
-pub fn getcwd() -> Path {
+pub fn getcwd() -> IoResult<Path> {
use c_str::CString;
let mut buf = [0 as c_char, ..BUF_BYTES];
unsafe {
if libc::getcwd(buf.as_mut_ptr(), buf.len() as libc::size_t).is_null() {
- panic!()
+ Err(IoError::last_error())
+ } else {
+ Ok(Path::new(CString::new(buf.as_ptr(), false)))
}
- Path::new(CString::new(buf.as_ptr(), false))
}
}
-/// Returns the current working directory as a Path.
+/// Returns the current working directory as a `Path`.
///
-/// # Failure
+/// # Errors
///
-/// Fails if the current working directory value is invalid.
-/// Possibles cases:
+/// Returns an `Err` if the current working directory value is invalid.
+/// Possible cases:
///
/// * Current directory does not exist.
/// * There are insufficient permissions to access the current directory.
+/// * The internal buffer is not large enough to hold the path.
///
/// # Example
///
@@ -125,23 +128,31 @@ pub fn getcwd() -> Path {
/// use std::os;
///
/// // We assume that we are in a valid directory like "C:\\Windows".
-/// let current_working_directory = os::getcwd();
+/// let current_working_directory = os::getcwd().unwrap();
/// println!("The current directory is {}", current_working_directory.display());
/// // C:\\Windows
/// ```
#[cfg(windows)]
-pub fn getcwd() -> Path {
+pub fn getcwd() -> IoResult<Path> {
use libc::DWORD;
use libc::GetCurrentDirectoryW;
+ use io::OtherIoError;
let mut buf = [0 as u16, ..BUF_BYTES];
unsafe {
if libc::GetCurrentDirectoryW(buf.len() as DWORD, buf.as_mut_ptr()) == 0 as DWORD {
- panic!();
+ return Err(IoError::last_error());
}
}
- Path::new(String::from_utf16(::str::truncate_utf16_at_nul(&buf))
- .expect("GetCurrentDirectoryW returned invalid UTF-16"))
+
+ match String::from_utf16(::str::truncate_utf16_at_nul(&buf)) {
+ Some(ref cwd) => Ok(Path::new(cwd)),
+ None => Err(IoError {
+ kind: OtherIoError,
+ desc: "GetCurrentDirectoryW returned invalid UTF-16",
+ detail: None,
+ }),
+ }
}
#[cfg(windows)]
@@ -829,20 +840,21 @@ pub fn tmpdir() -> Path {
///
/// // Assume we're in a path like /home/someuser
/// let rel_path = Path::new("..");
-/// let abs_path = os::make_absolute(&rel_path);
+/// let abs_path = os::make_absolute(&rel_path).unwrap();
/// println!("The absolute path is {}", abs_path.display());
/// // Prints "The absolute path is /home"
/// ```
// NB: this is here rather than in path because it is a form of environment
// querying; what it does depends on the process working directory, not just
// the input paths.
-pub fn make_absolute(p: &Path) -> Path {
+pub fn make_absolute(p: &Path) -> IoResult<Path> {
if p.is_absolute() {
- p.clone()
+ Ok(p.clone())
} else {
- let mut ret = getcwd();
- ret.push(p);
- ret
+ getcwd().map(|mut cwd| {
+ cwd.push(p);
+ cwd
+ })
}
}
@@ -1881,11 +1893,11 @@ mod tests {
fn test() {
assert!((!Path::new("test-path").is_absolute()));
- let cwd = getcwd();
+ let cwd = getcwd().unwrap();
debug!("Current working directory: {}", cwd.display());
- debug!("{}", make_absolute(&Path::new("test-path")).display());
- debug!("{}", make_absolute(&Path::new("/usr/bin")).display());
+ debug!("{}", make_absolute(&Path::new("test-path")).unwrap().display());
+ debug!("{}", make_absolute(&Path::new("/usr/bin")).unwrap().display());
}
#[test]
diff --git a/src/test/run-pass/process-spawn-with-unicode-params.rs b/src/test/run-pass/process-spawn-with-unicode-params.rs
index 490614ef12144..1c24210d6cd98 100644
--- a/src/test/run-pass/process-spawn-with-unicode-params.rs
+++ b/src/test/run-pass/process-spawn-with-unicode-params.rs
@@ -26,7 +26,7 @@ use std::path::Path;
fn main() {
let my_args = os::args();
- let my_cwd = os::getcwd();
+ let my_cwd = os::getcwd().unwrap();
let my_env = os::env();
let my_path = Path::new(os::self_exe_name().unwrap());
let my_dir = my_path.dir_path();
diff --git a/src/test/run-pass/tempfile.rs b/src/test/run-pass/tempfile.rs
index 476278405ca8a..bf47a516bc85c 100644
--- a/src/test/run-pass/tempfile.rs
+++ b/src/test/run-pass/tempfile.rs
@@ -123,7 +123,7 @@ fn test_rm_tempdir_close() {
// to depend on std
fn recursive_mkdir_rel() {
let path = Path::new("frob");
- let cwd = os::getcwd();
+ let cwd = os::getcwd().unwrap();
println!("recursive_mkdir_rel: Making: {} in cwd {} [{}]", path.display(),
cwd.display(), path.exists());
fs::mkdir_recursive(&path, io::USER_RWX);
@@ -141,7 +141,7 @@ fn recursive_mkdir_dot() {
fn recursive_mkdir_rel_2() {
let path = Path::new("./frob/baz");
- let cwd = os::getcwd();
+ let cwd = os::getcwd().unwrap();
println!("recursive_mkdir_rel_2: Making: {} in cwd {} [{}]", path.display(),
cwd.display(), path.exists());
fs::mkdir_recursive(&path, io::USER_RWX);
From 5de56b3ca1defd9206db8364ecef5f3fd8cc5b38 Mon Sep 17 00:00:00 2001
From: Barosl Lee <vcs@barosl.com>
Date: Tue, 11 Nov 2014 17:13:10 +0900
Subject: [PATCH 2/3] Make os::change_dir() return IoResult<()>
os::change_dir() returns bool, without a meaningful error message.
Change it to return IoResult<()> to indicate what IoError caused the
failure.
Fixes #16315.
[breaking-change]
---
src/libstd/os.rs | 29 +++++++++++++++--------------
src/test/run-pass/tempfile.rs | 2 +-
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index 971138c06fb67..cf22b8014ca7a 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -867,32 +867,33 @@ pub fn make_absolute(p: &Path) -> IoResult<Path> {
/// use std::path::Path;
///
/// let root = Path::new("/");
-/// assert!(os::change_dir(&root));
+/// assert!(os::change_dir(&root).is_ok());
/// println!("Successfully changed working directory to {}!", root.display());
/// ```
-pub fn change_dir(p: &Path) -> bool {
+pub fn change_dir(p: &Path) -> IoResult<()> {
return chdir(p);
#[cfg(windows)]
- fn chdir(p: &Path) -> bool {
- let p = match p.as_str() {
- Some(s) => {
- let mut p = s.utf16_units().collect::<Vec<u16>>();
- p.push(0);
- p
- }
- None => return false,
- };
+ fn chdir(p: &Path) -> IoResult<()> {
+ let mut p = p.as_str().unwrap().utf16_units().collect::<Vec<u16>>();
+ p.push(0);
+
unsafe {
- libc::SetCurrentDirectoryW(p.as_ptr()) != (0 as libc::BOOL)
+ match libc::SetCurrentDirectoryW(p.as_ptr()) != (0 as libc::BOOL) {
+ true => Ok(()),
+ false => Err(IoError::last_error()),
+ }
}
}
#[cfg(unix)]
- fn chdir(p: &Path) -> bool {
+ fn chdir(p: &Path) -> IoResult<()> {
p.with_c_str(|buf| {
unsafe {
- libc::chdir(buf) == (0 as c_int)
+ match libc::chdir(buf) == (0 as c_int) {
+ true => Ok(()),
+ false => Err(IoError::last_error()),
+ }
}
})
}
diff --git a/src/test/run-pass/tempfile.rs b/src/test/run-pass/tempfile.rs
index bf47a516bc85c..7400c52a73b4a 100644
--- a/src/test/run-pass/tempfile.rs
+++ b/src/test/run-pass/tempfile.rs
@@ -190,7 +190,7 @@ pub fn dont_double_panic() {
fn in_tmpdir(f: ||) {
let tmpdir = TempDir::new("test").ok().expect("can't make tmpdir");
- assert!(os::change_dir(tmpdir.path()));
+ assert!(os::change_dir(tmpdir.path()).is_ok());
f();
}
From b5286af703e33bd36744fe4cd5bb24f71dbb524e Mon Sep 17 00:00:00 2001
From: Barosl Lee <vcs@barosl.com>
Date: Wed, 12 Nov 2014 02:50:44 +0900
Subject: [PATCH 3/3] Make os::setenv() and os::unsetenv() panic if an error
occurs
These functions can fail if:
- EINVAL: The name is empty, or contains an '=' character
- ENOMEM: Insufficient memory
---
src/libstd/os.rs | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/libstd/os.rs b/src/libstd/os.rs
index cf22b8014ca7a..b898b9a2df43e 100644
--- a/src/libstd/os.rs
+++ b/src/libstd/os.rs
@@ -422,7 +422,9 @@ pub fn setenv<T: BytesContainer>(n: &str, v: T) {
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 {
+ panic!(IoError::last_error());
+ }
})
})
})
@@ -438,7 +440,9 @@ pub fn setenv<T: BytesContainer>(n: &str, v: T) {
unsafe {
with_env_lock(|| {
- libc::SetEnvironmentVariableW(n.as_ptr(), v.as_ptr());
+ if libc::SetEnvironmentVariableW(n.as_ptr(), v.as_ptr()) == 0 {
+ panic!(IoError::last_error());
+ }
})
}
}
@@ -453,7 +457,9 @@ pub fn unsetenv(n: &str) {
unsafe {
with_env_lock(|| {
n.with_c_str(|nbuf| {
- libc::funcs::posix01::unistd::unsetenv(nbuf);
+ if libc::funcs::posix01::unistd::unsetenv(nbuf) != 0 {
+ panic!(IoError::last_error());
+ }
})
})
}
@@ -465,11 +471,14 @@ pub fn unsetenv(n: &str) {
n.push(0);
unsafe {
with_env_lock(|| {
- libc::SetEnvironmentVariableW(n.as_ptr(), ptr::null());
+ if libc::SetEnvironmentVariableW(n.as_ptr(), ptr::null()) == 0 {
+ panic!(IoError::last_error());
+ }
})
}
}
- _unsetenv(n);
+
+ _unsetenv(n)
}
/// Parses input according to platform conventions for the `PATH`