From e03ab6b554873b5ef6a7b9cb55e99a3b80a30cfb Mon Sep 17 00:00:00 2001 From: Connor E <38229097+c-edw@users.noreply.github.com> Date: Tue, 1 May 2018 12:07:23 +0100 Subject: [PATCH 1/3] mkdir: Silently fail in recursive mode if unable to create directories. --- src/mkdir/mkdir.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/mkdir/mkdir.rs b/src/mkdir/mkdir.rs index ec07d36e8..309b6b50e 100644 --- a/src/mkdir/mkdir.rs +++ b/src/mkdir/mkdir.rs @@ -94,10 +94,10 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { let path = Path::new(dir); if recursive { let mut pathbuf = PathBuf::new(); - for component in path.components() { - pathbuf.push(component.as_os_str()); - if !path.is_dir() { - status |= mkdir(pathbuf.as_path(), mode, verbose); + if !path.is_dir() { + for component in path.components() { + pathbuf.push(component.as_os_str()); + mkdir(pathbuf.as_path(), recursive, mode, verbose); } } } else { @@ -110,11 +110,11 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { ); status = 1; } else { - status |= mkdir(path, mode, verbose); + status |= mkdir(path, recursive, mode, verbose); } } None => { - status |= mkdir(path, mode, verbose); + status |= mkdir(path, recursive, mode, verbose); } } } @@ -125,9 +125,11 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { /** * Wrapper to catch errors, return 1 if failed */ -fn mkdir(path: &Path, mode: u16, verbose: bool) -> i32 { +fn mkdir(path: &Path, recursive: bool, mode: u16, verbose: bool) -> i32 { if let Err(e) = fs::create_dir(path) { - show_info!("{}: {}", path.display(), e.to_string()); + if !recursive { + show_info!("{}: {}", path.display(), e.to_string()); + } return 1; } From 9d5631228aec3f8a477c452e29042d24eff03060 Mon Sep 17 00:00:00 2001 From: Connor E <38229097+c-edw@users.noreply.github.com> Date: Tue, 1 May 2018 12:42:11 +0100 Subject: [PATCH 2/3] mkdir: Use std create_dir_all for recursive operations. --- src/mkdir/mkdir.rs | 21 ++++++++++----------- tests/test_mkdir.rs | 1 + 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/mkdir/mkdir.rs b/src/mkdir/mkdir.rs index 309b6b50e..15d9d2484 100644 --- a/src/mkdir/mkdir.rs +++ b/src/mkdir/mkdir.rs @@ -93,13 +93,7 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { for dir in &dirs { let path = Path::new(dir); if recursive { - let mut pathbuf = PathBuf::new(); - if !path.is_dir() { - for component in path.components() { - pathbuf.push(component.as_os_str()); - mkdir(pathbuf.as_path(), recursive, mode, verbose); - } - } + status |= mkdir(path, recursive, mode, verbose); } else { match path.parent() { Some(parent) => { @@ -126,11 +120,16 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { * Wrapper to catch errors, return 1 if failed */ fn mkdir(path: &Path, recursive: bool, mode: u16, verbose: bool) -> i32 { - if let Err(e) = fs::create_dir(path) { - if !recursive { - show_info!("{}: {}", path.display(), e.to_string()); + if recursive { + if let Err(e) = fs::create_dir_all(path) { + show_info!("cannot create directory '{}': {}", path.display(), e.to_string()); + return 1; + } + } else { + if let Err(e) = fs::create_dir(path) { + show_info!("{}: {}", path.display(), e.to_string()); + return 1; } - return 1; } if verbose { diff --git a/tests/test_mkdir.rs b/tests/test_mkdir.rs index 91d4881f0..3e5ec4b06 100644 --- a/tests/test_mkdir.rs +++ b/tests/test_mkdir.rs @@ -53,6 +53,7 @@ fn test_mkdir_dup_file() { let scene = TestScenario::new(util_name!()); scene.fixtures.touch(TEST_FILE7); scene.ucmd().arg(TEST_FILE7).fails(); + // mkdir should fail for a file even if -p is specified. scene.ucmd().arg("-p").arg(TEST_FILE7).fails(); } From 62632faa633ba493b3a14f59e680ed5dcfe8382f Mon Sep 17 00:00:00 2001 From: Connor E <38229097+c-edw@users.noreply.github.com> Date: Thu, 3 May 2018 08:40:01 +0100 Subject: [PATCH 3/3] Clean up with changes from Arcterus. --- src/mkdir/mkdir.rs | 41 ++++++++++++++--------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/src/mkdir/mkdir.rs b/src/mkdir/mkdir.rs index 15d9d2484..231ce3b93 100644 --- a/src/mkdir/mkdir.rs +++ b/src/mkdir/mkdir.rs @@ -92,26 +92,19 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { let empty = Path::new(""); for dir in &dirs { let path = Path::new(dir); - if recursive { - status |= mkdir(path, recursive, mode, verbose); - } else { - match path.parent() { - Some(parent) => { - if parent != empty && !parent.exists() { - show_info!( - "cannot create directory '{}': No such file or directory", - path.display() - ); - status = 1; - } else { - status |= mkdir(path, recursive, mode, verbose); - } - } - None => { - status |= mkdir(path, recursive, mode, verbose); + if !recursive { + if let Some(parent) = path.parent() { + if parent != empty && !parent.exists() { + show_info!( + "cannot create directory '{}': No such file or directory", + path.display() + ); + status = 1; + continue; } } } + status |= mkdir(path, recursive, mode, verbose); } status } @@ -120,16 +113,10 @@ fn exec(dirs: Vec, recursive: bool, mode: u16, verbose: bool) -> i32 { * Wrapper to catch errors, return 1 if failed */ fn mkdir(path: &Path, recursive: bool, mode: u16, verbose: bool) -> i32 { - if recursive { - if let Err(e) = fs::create_dir_all(path) { - show_info!("cannot create directory '{}': {}", path.display(), e.to_string()); - return 1; - } - } else { - if let Err(e) = fs::create_dir(path) { - show_info!("{}: {}", path.display(), e.to_string()); - return 1; - } + let create_dir = if recursive { fs::create_dir_all } else { fs::create_dir }; + if let Err(e) = create_dir(path) { + show_info!("{}: {}", path.display(), e.to_string()); + return 1; } if verbose {