mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

auto merge of #18870 : barosl/rust/os-ioresult, r=alexcrichton

Browse Source 
Make old-fashioned functions in the `std::os` module utilize `IoResult`.

I'm still investigating the possibility to include more functions in this pull request. Currently, it covers `getcwd()`, `make_absolute()`, and `change_dir()`. The issues covered by this PR are #16946 and #16315.

A few concerns:

- Should we provide `OsError` in distinction from `IoError`? I'm saying this because in Python, those two are distinguished. One advantage that we keep using `IoError` is that we can make the error cascade down other functions whose return type also includes `IoError`. An example of such functions is `std::io::TempDir::new_in()`, which uses `os::make_absolute()` as well as returns `IoResult<TempDir>`.
- `os::getcwd()` uses an internal buffer whose size is 2048 bytes, which is passed to `getcwd(3)`. There is no upper limitation of file paths in the POSIX standard, but typically it is set to 4096 bytes such as in Linux. Should we increase the buffer size? One thing that makes me nervous is that the size of 2048 bytes already seems a bit excessive, thinking that in normal cases, there would be no filenames that even exceeds 512 bytes.

Fixes #16946.
Fixes #16315.

Any ideas are welcomed. Thanks!
This commit is contained in:
bors 2014-11-18 21:56:58 +00:00
commit 1628b98183
12 changed files with 86 additions and 63 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/librustc/metadata/filesearch.rs
View File

@ -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) {

2
src/librustc/plugin/load.rs
View File

@ -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,

4
src/librustc/session/mod.rs
View File

@ -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()),

4
src/librustc_back/archive.rs
View File

@ -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]);

2
src/librustc_back/fs.rs
View File

@ -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) {

6
src/librustc_back/rpath.rs
View File

@ -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()
}

4
src/libstd/io/process.rs
View File

@ -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();

3
src/libstd/io/tempfile.rs
View File

@ -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;

2
src/libstd/io/test.rs
View File

@ -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;

112
src/libstd/os.rs
View File

@ -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)]
@ -411,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());
}
})
})
})
@ -427,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());
}
})
}
}
@ -442,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());
}
})
})
}
@ -454,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`
@ -829,20 +849,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
})
}
}
@ -855,32 +876,33 @@ pub fn make_absolute(p: &Path) -> 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()),
}
}
})
}
@ -1881,11 +1903,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]

2
src/test/run-pass/process-spawn-with-unicode-params.rs
View File

@ -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();

6
src/test/run-pass/tempfile.rs
View File

@ -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);
@ -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();
}