From d045ce7b87af0fb0730ccf5291c11d28a5382254 Mon Sep 17 00:00:00 2001 From: Tim Chevalier Date: Sun, 28 Apr 2013 21:25:35 -0700 Subject: [PATCH] core: Use a better termination condition in os::mkdir_recursive Instead of checking whether the parent is "." or "/", check the number of components. Also, more tests. --- src/libcore/os.rs | 38 +++++++++++++----------- src/libstd/tempfile.rs | 65 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 80 insertions(+), 23 deletions(-) diff --git a/src/libcore/os.rs b/src/libcore/os.rs index b81209d35cf..f1962eeaa23 100644 --- a/src/libcore/os.rs +++ b/src/libcore/os.rs @@ -643,20 +643,22 @@ pub fn make_dir(p: &Path, mode: c_int) -> bool { /// Returns true iff creation /// succeeded. Also creates all intermediate subdirectories /// if they don't already exist, giving all of them the same mode. + +// tjc: if directory exists but with different permissions, +// should we return false? pub fn mkdir_recursive(p: &Path, mode: c_int) -> bool { if path_is_dir(p) { return true; } - let parent = p.dir_path(); - debug!("mkdir_recursive: parent = %s", - parent.to_str()); - if parent.to_str() == ~"." - || parent.to_str() == ~"/" { // !!! + else if p.components.is_empty() { + return false; + } + else if p.components.len() == 1 { // No parent directories to create - path_is_dir(&parent) && make_dir(p, mode) + path_is_dir(p) || make_dir(p, mode) } else { - mkdir_recursive(&parent, mode) && make_dir(p, mode) + mkdir_recursive(&p.pop(), mode) && make_dir(p, mode) } } @@ -1267,6 +1269,8 @@ mod tests { use run; use str; use vec; + use libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; + #[test] pub fn last_os_error() { @@ -1490,16 +1494,16 @@ mod tests { } #[test] - fn recursive_mkdir_ok() { - use libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; - - let root = os::tmpdir(); - let path = "xy/z/zy"; - let nested = root.push(path); - assert!(os::mkdir_recursive(&nested, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); - assert!(os::path_is_dir(&root.push("xy"))); - assert!(os::path_is_dir(&root.push("xy/z"))); - assert!(os::path_is_dir(&nested)); + fn recursive_mkdir_slash() { + let path = Path("/"); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); } + #[test] + fn recursive_mkdir_empty() { + let path = Path(""); + assert!(!os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + } + + // More recursive_mkdir tests are in std::tempfile } diff --git a/src/libstd/tempfile.rs b/src/libstd/tempfile.rs index 846a9aec153..eec91b68454 100644 --- a/src/libstd/tempfile.rs +++ b/src/libstd/tempfile.rs @@ -23,9 +23,62 @@ pub fn mkdtemp(tmpdir: &Path, suffix: &str) -> Option { None } -#[test] -fn test_mkdtemp() { - let p = mkdtemp(&Path("."), "foobar").unwrap(); - os::remove_dir(&p); - assert!(str::ends_with(p.to_str(), "foobar")); -} +#[cfg(test)] +mod tests { + use tempfile::mkdtemp; + use tempfile; + + #[test] + fn test_mkdtemp() { + let p = mkdtemp(&Path("."), "foobar").unwrap(); + os::remove_dir(&p); + assert!(str::ends_with(p.to_str(), "foobar")); + } + + // Ideally these would be in core::os but then core would need + // to depend on std + #[test] + fn recursive_mkdir_rel() { + use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; + use core::os; + + let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel"); + os::change_dir(&root); + let path = Path("frob"); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + } + + #[test] + fn recursive_mkdir_dot() { + use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; + use core::os; + + let dot = Path("."); + assert!(os::mkdir_recursive(&dot, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + let dotdot = Path(".."); + assert!(os::mkdir_recursive(&dotdot, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + } + + #[test] + fn recursive_mkdir_rel_2() { + use core::libc::consts::os::posix88::{S_IRUSR, S_IWUSR, S_IXUSR}; + use core::os; + + let root = mkdtemp(&os::tmpdir(), "temp").expect("recursive_mkdir_rel_2"); + os::change_dir(&root); + let path = Path("./frob/baz"); + debug!("...Making: %s in cwd %s", path.to_str(), os::getcwd().to_str()); + assert!(os::mkdir_recursive(&path, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path)); + assert!(os::path_is_dir(&path.pop())); + let path2 = Path("quux/blat"); + debug!("Making: %s in cwd %s", path2.to_str(), os::getcwd().to_str()); + assert!(os::mkdir_recursive(&path2, (S_IRUSR | S_IWUSR | S_IXUSR) as i32)); + assert!(os::path_is_dir(&path2)); + assert!(os::path_is_dir(&path2.pop())); + } + +} \ No newline at end of file