From fd5310411e6ba3b0041e3241887f3d7cd0c28f3d Mon Sep 17 00:00:00 2001 From: kimono-koans <32370782+kimono-koans@users.noreply.github.com> Date: Fri, 14 Jan 2022 17:39:56 -0600 Subject: [PATCH] ls: Fix device display (#2855) --- src/uu/ls/src/ls.rs | 119 +++++++++++++++++++++++++++++++-------- tests/by-util/test_ls.rs | 64 ++++++++++++++++++++- 2 files changed, 160 insertions(+), 23 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6e6b1c559..fd3e41adb 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -319,6 +319,10 @@ struct PaddingCollection { longest_group_len: usize, longest_context_len: usize, longest_size_len: usize, + #[cfg(unix)] + longest_major_len: usize, + #[cfg(unix)] + longest_minor_len: usize, } impl Config { @@ -1560,18 +1564,28 @@ fn display_dir_entry_size( entry: &PathData, config: &Config, out: &mut BufWriter, -) -> (usize, usize, usize, usize, usize) { +) -> (usize, usize, usize, usize, usize, usize, usize) { // TODO: Cache/memorize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md(out) { + let (size_len, major_len, minor_len) = match display_size_or_rdev(md, config) { + SizeOrDeviceId::Device(major, minor) => ( + (major.len() + minor.len() + 2usize), + major.len(), + minor.len(), + ), + SizeOrDeviceId::Size(size) => (size.len(), 0usize, 0usize), + }; ( display_symlink_count(md).len(), display_uname(md, config).len(), display_group(md, config).len(), - display_size_or_rdev(md, config).len(), + size_len, + major_len, + minor_len, display_inode(md).len(), ) } else { - (0, 0, 0, 0, 0) + (0, 0, 0, 0, 0, 0, 0) } } @@ -1608,7 +1622,9 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter { + let _ = write!(out, " {}", pad_left(&size, padding.longest_size_len),); + } + SizeOrDeviceId::Device(major, minor) => { + let _ = write!( + out, + " {}, {}", + pad_left( + &major, + #[cfg(not(unix))] + 0usize, + #[cfg(unix)] + padding.longest_major_len.max( + padding + .longest_size_len + .saturating_sub(padding.longest_minor_len.saturating_add(2usize)) + ) + ), + pad_left( + &minor, + #[cfg(not(unix))] + 0usize, + #[cfg(unix)] + padding.longest_minor_len, + ), + ); + } + }; + let dfn = display_file_name(item, config, None, 0, out).contents; - let _ = writeln!( - out, - " {} {} {}", - pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), - display_date(md, config), - dfn, - ); + let _ = writeln!(out, " {} {}", display_date(md, config), dfn); } else { // this 'else' is expressly for the case of a dangling symlink/restricted file #[cfg(unix)] @@ -2112,19 +2171,35 @@ fn format_prefixed(prefixed: NumberPrefix) -> String { } } -fn display_size_or_rdev(metadata: &Metadata, config: &Config) -> String { - #[cfg(unix)] +#[allow(dead_code)] +enum SizeOrDeviceId { + Size(String), + Device(String, String), +} + +fn display_size_or_rdev(metadata: &Metadata, config: &Config) -> SizeOrDeviceId { + #[cfg(any(target_os = "macos", target_os = "ios"))] + { + let ft = metadata.file_type(); + if ft.is_char_device() || ft.is_block_device() { + let dev: u64 = metadata.rdev(); + let major = (dev >> 24) as u8; + let minor = (dev & 0xff) as u8; + return SizeOrDeviceId::Device(major.to_string(), minor.to_string()); + } + } + #[cfg(target_os = "linux")] { let ft = metadata.file_type(); if ft.is_char_device() || ft.is_block_device() { let dev: u64 = metadata.rdev(); let major = (dev >> 8) as u8; - let minor = dev as u8; - return format!("{}, {}", major, minor,); + let minor = (dev & 0xff) as u8; + return SizeOrDeviceId::Device(major.to_string(), minor.to_string()); } } - display_size(metadata.len(), config) + SizeOrDeviceId::Size(display_size(metadata.len(), config)) } fn display_size(size: u64, config: &Config) -> String { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3b3316338..b5d49337d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -56,8 +56,70 @@ fn test_ls_ordering() { .stdout_matches(&Regex::new("some-dir1:\\ntotal 0").unwrap()); } +//#[cfg(all(feature = "mknod"))] +#[test] +fn test_ls_devices() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + at.mkdir("some-dir1"); + + // Regex tests correct device ID and correct (no pad) spacing for a single file + #[cfg(any(target_os = "macos", target_os = "ios"))] + { + scene + .ucmd() + .arg("-al") + .arg("/dev/null") + .succeeds() + .stdout_matches(&Regex::new("[^ ] 3, 2 [^ ]").unwrap()); + } + + #[cfg(target_os = "linux")] + { + scene + .ucmd() + .arg("-al") + .arg("/dev/null") + .succeeds() + .stdout_matches(&Regex::new("[^ ] 1, 3 [^ ]").unwrap()); + } + + // Regex tests alignment against a file (stdout is a link to a tty) + #[cfg(unix)] + { + let res = scene + .ucmd() + .arg("-alL") + .arg("/dev/null") + .arg("/dev/stdout") + .succeeds(); + + let null_len = String::from_utf8(res.stdout().to_owned()) + .ok() + .unwrap() + .lines() + .next() + .unwrap() + .strip_suffix("/dev/null") + .unwrap() + .len(); + + let stdout_len = String::from_utf8(res.stdout().to_owned()) + .ok() + .unwrap() + .lines() + .nth(1) + .unwrap() + .strip_suffix("/dev/stdout") + .unwrap() + .len(); + + assert_eq!(stdout_len, null_len); + } +} + +#[cfg(all(feature = "chmod"))] #[test] -#[cfg(feature = "chmod")] fn test_ls_io_errors() { let scene = TestScenario::new(util_name!()); let at = &scene.fixtures;