From 15efba54c52db99530264a40aa766865247ff786 Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+electricboogie@users.noreply.github.com> Date: Sun, 16 Jan 2022 10:20:50 -0600 Subject: [PATCH] Use dir_entry metadata for dereferenced bad fds to match GNU, add comments, clippy lints --- build.rs | 4 ++-- src/uu/ls/src/ls.rs | 30 ++++++++++++++++++---------- tests/by-util/test_ls.rs | 43 +++++++++++++++++++++------------------- 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/build.rs b/build.rs index 293d2e65f..03563730f 100644 --- a/build.rs +++ b/build.rs @@ -83,7 +83,7 @@ pub fn main() { mf.write_all( format!( "\tmap.insert(\"{k}\", ({krate}::uumain, {krate}::uu_app));\n", - k = krate[override_prefix.len()..].to_string(), + k = krate[override_prefix.len()..].to_owned(), krate = krate ) .as_bytes(), @@ -92,7 +92,7 @@ pub fn main() { tf.write_all( format!( "#[path=\"{dir}/test_{k}.rs\"]\nmod test_{k};\n", - k = krate[override_prefix.len()..].to_string(), + k = krate[override_prefix.len()..].to_owned(), dir = util_tests_dir, ) .as_bytes(), diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 226dcd1bf..2da4fa328 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -207,9 +207,10 @@ impl Display for LsError { } _ => match errno { 9i32 => { + // only should ever occur on a read_dir on a bad fd write!( f, - "cannot access '{}': Bad file descriptor", + "cannot open directory '{}': Bad file descriptor", p.to_string_lossy(), ) } @@ -1354,18 +1355,24 @@ impl PathData { Err(err) => { let _ = out.flush(); let errno = err.raw_os_error().unwrap_or(1i32); - // Wait to enter "directory" to print error for a bad fd + // Wait to enter "directory" to print error for any bad fd if self.must_dereference && errno.eq(&9i32) { - if let Ok(md) = get_metadata(&self.p_buf, false) { - Some(md) - } else { - show!(LsError::IOErrorContext(err, self.p_buf.clone(),)); - None + if let Some(parent) = self.p_buf.parent() { + if let Ok(read_dir) = fs::read_dir(parent) { + // this dir_entry metadata is different from the metadata call on the path + let res = read_dir + .filter_map(|x| x.ok()) + .filter(|x| self.p_buf.eq(&x.path())) + .filter_map(|x| x.metadata().ok()) + .next(); + if let Some(md) = res { + return Some(md); + } + } } - } else { - show!(LsError::IOErrorContext(err, self.p_buf.clone(),)); - None } + show!(LsError::IOErrorContext(err, self.p_buf.clone(),)); + None } Ok(md) => Some(md), }, @@ -1565,7 +1572,8 @@ fn enter_directory( let res = PathData::new(dir_entry.path(), Some(Ok(ft)), None, config, false); // metadata returned from a DirEntry matches GNU metadata for // non-dereferenced files, and is *different* from the - // metadata call on the path, see, for example, bad fds + // metadata call on the path, see, for example, bad fds, + // so we use here when we will need metadata later anyway if !res.must_dereference && ((config.format == Format::Long) || (config.sort == Sort::Name) diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 85b336a26..bc840173f 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -170,7 +170,7 @@ fn test_ls_io_errors() { .stderr_contains("Permission denied") .stdout_contains("some-dir4"); - // test we don't double print on dangling link metadata errors + // don't double print on dangling link metadata errors scene .ucmd() .arg("-iRL") @@ -188,26 +188,29 @@ fn test_ls_io_errors() { let rv1 = dup2(fd1, fd2); let rv2 = close(fd1); - scene - .ucmd() - .arg("-alR") - .arg(format!("/dev/fd/{}", fd2)) - .fails() - .stderr_contains(format!( - "cannot access '/dev/fd/{}': Bad file descriptor", - fd2 - )) - .stdout_does_not_contain(format!("{}:\n", fd2)); - - scene - .ucmd() - .arg("-RiL") - .arg(format!("/dev/fd/{}", fd2)) - .fails() - .stderr_contains(format!("cannot access '/dev/fd/{}': Bad file descriptor", fd2)) - // test we only print bad fd error once - .stderr_does_not_contain(format!("ls: cannot access '/dev/fd/{fd}': Bad file descriptor\nls: cannot access '/dev/fd/{fd}': Bad file descriptor", fd = fd2)); + // dup and close work on the mac, but doesn't work in some linux containers + // so check to see that return values are non-error before proceeding + if rv1.is_ok() && rv2.is_ok() { + scene + .ucmd() + .arg("-alR") + .arg(format!("/dev/fd/{}", fd = fd2)) + .fails() + .stderr_contains(format!( + "cannot open directory '/dev/fd/{fd}': Bad file descriptor", + fd = fd2 + )) + .stdout_does_not_contain(format!("{fd}:\n", fd = fd2)); + scene + .ucmd() + .arg("-RiL") + .arg(format!("/dev/fd/{fd}", fd = fd2)) + .fails() + .stderr_contains(format!("cannot open directory '/dev/fd/{fd}': Bad file descriptor", fd = fd2)) + // don't double print bad fd errors + .stderr_does_not_contain(format!("ls: cannot open directory '/dev/fd/{fd}': Bad file descriptor\nls: cannot open directory '/dev/fd/{fd}': Bad file descriptor", fd = fd2)); + } let _ = close(fd2); } }