Merge pull request #9078 from Alonely0/stat_fix

fix(stat): Collection of fixes for stat(1)

Author: sylvestre

.vscode/cspell.dictionaries/jargon.wordlist.txt

@@ -86,6 +86,7 @@ listxattr
 llistxattr
 lossily
 lstat
+makedev
 mebi
 mebibytes
 mergeable

src/uu/mknod/Cargo.toml

@@ -21,7 +21,7 @@ path = "src/mknod.rs"
 [dependencies]
 clap = { workspace = true }
 libc = { workspace = true }
-uucore = { workspace = true, features = ["mode"] }
+uucore = { workspace = true, features = ["mode", "fs"] }
 fluent = { workspace = true }
 
 [features]

src/uu/mknod/src/mknod.rs

@@ -13,6 +13,7 @@ use std::ffi::CString;
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError, UUsageError, set_exit_code};
 use uucore::format_usage;
+use uucore::fs::makedev;
 use uucore::translate;
 
 const MODE_RW_UGO: mode_t = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
@@ -26,12 +27,6 @@ mod options {
     pub const CONTEXT: &str = "context";
 }
 
-#[inline(always)]
-fn makedev(maj: u64, min: u64) -> dev_t {
-    // pick up from <sys/sysmacros.h>
-    ((min & 0xff) | ((maj & 0xfff) << 8) | ((min & !0xff) << 12) | ((maj & !0xfff) << 32)) as dev_t
-}
-
 #[derive(Clone, PartialEq)]
 enum FileType {
     Block,
@@ -145,7 +140,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                 translate!("mknod-error-fifo-no-major-minor"),
             ));
         }
-        (_, Some(&major), Some(&minor)) => makedev(major, minor),
+        (_, Some(&major), Some(&minor)) => makedev(major as _, minor as _),
         _ => {
             return Err(UUsageError::new(
                 1,

src/uu/stat/src/stat.rs

@@ -9,7 +9,7 @@ use uucore::translate;
 
 use clap::builder::ValueParser;
 use uucore::display::Quotable;
-use uucore::fs::display_permissions;
+use uucore::fs::{display_permissions, major, minor};
 use uucore::fsext::{
     FsMeta, MetadataTimeField, StatFs, metadata_get_time, pretty_filetype, pretty_fstype,
     read_fs_list, statfs,
@@ -70,6 +70,8 @@ struct Flags {
     space: bool,
     sign: bool,
     group: bool,
+    major: bool,
+    minor: bool,
 }
 
 /// checks if the string is within the specified bound,
@@ -739,7 +741,6 @@ impl Stater {
             return Ok(Token::Char('%'));
         }
         if chars[*i] == '%' {
-            *i += 1;
             return Ok(Token::Char('%'));
         }
 
@@ -794,13 +795,14 @@ impl Stater {
             if let Some(&next_char) = chars.get(*i + 1) {
                 if (chars[*i] == 'H' || chars[*i] == 'L') && (next_char == 'd' || next_char == 'r')
                 {
-                    let specifier = format!("{}{next_char}", chars[*i]);
+                    flag.major = chars[*i] == 'H';
+                    flag.minor = chars[*i] == 'L';
                     *i += 1;
                     return Ok(Token::Directive {
                         flag,
                         width,
                         precision,
-                        format: specifier.chars().next().unwrap(),
+                        format: next_char,
                     });
                 }
             }
@@ -908,6 +910,28 @@ impl Stater {
         Ok(tokens)
     }
 
+    fn populate_mount_list() -> UResult<Vec<OsString>> {
+        let mut mount_list = read_fs_list()
+            .map_err(|e| {
+                USimpleError::new(
+                    e.code(),
+                    StatError::CannotReadFilesystem {
+                        error: e.to_string(),
+                    }
+                    .to_string(),
+                )
+            })?
+            .iter()
+            .map(|mi| mi.mount_dir.clone())
+            .collect::<Vec<_>>();
+
+        // Reverse sort. The longer comes first.
+        mount_list.sort();
+        mount_list.reverse();
+
+        Ok(mount_list)
+    }
+
     fn new(matches: &ArgMatches) -> UResult<Self> {
         let files: Vec<OsString> = matches
             .get_many::<OsString>(options::FILES)
@@ -938,27 +962,16 @@ impl Stater {
         let default_dev_tokens =
             Self::generate_tokens(&Self::default_format(show_fs, terse, true), use_printf)?;
 
-        let mount_list = if show_fs {
-            // mount points aren't displayed when showing filesystem information
+        // mount points aren't displayed when showing filesystem information, or
+        // whenever the format string does not request the mount point.
+        let mount_list = if show_fs
+            || !default_tokens
+                .iter()
+                .any(|tok| matches!(tok, Token::Directive { format: 'm', .. }))
+        {
             None
         } else {
-            let mut mount_list = read_fs_list()
-                .map_err(|e| {
-                    USimpleError::new(
-                        e.code(),
-                        StatError::CannotReadFilesystem {
-                            error: e.to_string(),
-                        }
-                        .to_string(),
-                    )
-                })?
-                .iter()
-                .map(|mi| mi.mount_dir.clone())
-                .collect::<Vec<_>>();
-            // Reverse sort. The longer comes first.
-            mount_list.sort();
-            mount_list.reverse();
-            Some(mount_list)
+            Some(Self::populate_mount_list()?)
         };
 
         Ok(Self {
@@ -1052,6 +1065,8 @@ impl Stater {
                         }
                     }
                     // device number in decimal
+                    'd' if flag.major => OutputType::Unsigned(major(meta.dev() as _) as u64),
+                    'd' if flag.minor => OutputType::Unsigned(minor(meta.dev() as _) as u64),
                     'd' => OutputType::Unsigned(meta.dev()),
                     // device number in hex
                     'D' => OutputType::UnsignedHex(meta.dev()),
@@ -1090,10 +1105,10 @@ impl Stater {
                     's' => OutputType::Integer(meta.len() as i64),
                     // major device type in hex, for character/block device special
                     // files
-                    't' => OutputType::UnsignedHex(meta.rdev() >> 8),
+                    't' => OutputType::UnsignedHex(major(meta.rdev() as _) as u64),
                     // minor device type in hex, for character/block device special
                     // files
-                    'T' => OutputType::UnsignedHex(meta.rdev() & 0xff),
+                    'T' => OutputType::UnsignedHex(minor(meta.rdev() as _) as u64),
                     // user ID of owner
                     'u' => OutputType::Unsigned(meta.uid() as u64),
                     // user name of owner
@@ -1136,15 +1151,10 @@ impl Stater {
                             .map_or((0, 0), system_time_to_sec);
                         OutputType::Float(sec as f64 + nsec as f64 / 1_000_000_000.0)
                     }
-                    'R' => {
-                        let major = meta.rdev() >> 8;
-                        let minor = meta.rdev() & 0xff;
-                        OutputType::Str(format!("{major},{minor}"))
-                    }
+                    'R' => OutputType::UnsignedHex(meta.rdev()),
+                    'r' if flag.major => OutputType::Unsigned(major(meta.rdev() as _) as u64),
+                    'r' if flag.minor => OutputType::Unsigned(minor(meta.rdev() as _) as u64),
                     'r' => OutputType::Unsigned(meta.rdev()),
-                    'H' => OutputType::Unsigned(meta.rdev() >> 8), // Major in decimal
-                    'L' => OutputType::Unsigned(meta.rdev() & 0xff), // Minor in decimal
-
                     _ => OutputType::Unknown,
                 };
                 print_it(&output, flag, width, precision);
@@ -1269,7 +1279,7 @@ impl Stater {
         } else {
             let device_line = if show_dev_type {
                 format!(
-                    "{}: %Dh/%dd\t{}: %-10i  {}: %-5h {} {}: %t,%T\n",
+                    "{}: %Hd,%Ld\t{}: %-10i  {}: %-5h {} {}: %t,%T\n",
                     translate!("stat-word-device"),
                     translate!("stat-word-inode"),
                     translate!("stat-word-links"),
@@ -1278,7 +1288,7 @@ impl Stater {
                 )
             } else {
                 format!(
-                    "{}: %Dh/%dd\t{}: %-10i  {}: %h\n",
+                    "{}: %Hd,%Ld\t{}: %-10i  {}: %h\n",
                     translate!("stat-word-device"),
                     translate!("stat-word-inode"),
                     translate!("stat-word-links")

src/uucore/src/lib/features/fs.rs

@@ -13,6 +13,8 @@ use libc::{
     S_IRUSR, S_ISGID, S_ISUID, S_ISVTX, S_IWGRP, S_IWOTH, S_IWUSR, S_IXGRP, S_IXOTH, S_IXUSR,
     mkfifo, mode_t,
 };
+#[cfg(all(unix, not(target_os = "redox")))]
+pub use libc::{major, makedev, minor};
 use std::collections::HashSet;
 use std::collections::VecDeque;
 use std::env;
@@ -839,6 +841,24 @@ pub fn make_fifo(path: &Path) -> std::io::Result<()> {
     }
 }
 
+// Redox's libc appears not to include the following utilities
+
+#[cfg(target_os = "redox")]
+pub fn major(dev: libc::dev_t) -> libc::c_uint {
+    (((dev >> 8) & 0xFFF) | ((dev >> 32) & 0xFFFFF000)) as _
+}
+
+#[cfg(target_os = "redox")]
+pub fn minor(dev: libc::dev_t) -> libc::c_uint {
+    ((dev & 0xFF) | ((dev >> 12) & 0xFFFFF00)) as _
+}
+
+#[cfg(target_os = "redox")]
+pub fn makedev(maj: libc::c_uint, min: libc::c_uint) -> libc::dev_t {
+    let [maj, min] = [maj as libc::dev_t, min as libc::dev_t];
+    (min & 0xff) | ((maj & 0xfff) << 8) | ((min & !0xff) << 12) | ((maj & !0xfff) << 32)
+}
+
 #[cfg(test)]
 mod tests {
     // Note this useful idiom: importing names from outer (for mod tests) scope.

tests/by-util/test_stat.rs

@@ -9,6 +9,9 @@ use uutests::unwrap_or_return;
 use uutests::util::{TestScenario, expected_result};
 use uutests::util_name;
 
+use std::fs::metadata;
+use std::os::unix::fs::MetadataExt;
+
 #[test]
 fn test_invalid_arg() {
     new_ucmd!().arg("--definitely-invalid").fails_with_code(1);
@@ -567,3 +570,65 @@ fn test_mount_point_combined_with_other_specifiers() {
         "Should print mount point, file name, and size"
     );
 }
+
+#[cfg(unix)]
+#[test]
+fn test_percent_escaping() {
+    let ts = TestScenario::new(util_name!());
+    let result = ts
+        .ucmd()
+        .args(&["--printf", "%%%m%%m%m%%%", "/bin/sh"])
+        .succeeds();
+    assert_eq!(result.stdout_str(), "%/%m/%%");
+}
+
+#[cfg(unix)]
+#[test]
+fn test_correct_metadata() {
+    use uucore::fs::{major, minor};
+
+    let ts = TestScenario::new(util_name!());
+    let parse = |(i, str): (usize, &str)| {
+        // Some outputs (%[fDRtT]) are in hex; they're redundant, but we might
+        // as well also test case conversion.
+        let radix = if matches!(i, 2 | 10 | 14..) { 16 } else { 10 };
+        i128::from_str_radix(str, radix)
+    };
+    for device in ["/", "/dev/null"] {
+        let metadata = metadata(device).unwrap();
+        // We avoid time vals because of fs race conditions, especially with
+        // access time and status time (this previously killed an otherwise
+        // perfect 11-hour-long CI run...). The large number of as-casts is
+        // due to inconsistencies on some platforms (read: BSDs), and we use
+        // i128 as a lowest-common denominator.
+        let test_str = "%u %g %f %b %s %h %i %d %Hd %Ld %D %r %Hr %Lr %R %t %T";
+        let expected = [
+            metadata.uid() as _,
+            metadata.gid() as _,
+            metadata.mode() as _,
+            metadata.blocks() as _,
+            metadata.size() as _,
+            metadata.nlink() as _,
+            metadata.ino() as _,
+            metadata.dev() as _,
+            major(metadata.dev() as _) as _,
+            minor(metadata.dev() as _) as _,
+            metadata.dev() as _,
+            metadata.rdev() as _,
+            major(metadata.rdev() as _) as _,
+            minor(metadata.rdev() as _) as _,
+            metadata.rdev() as _,
+            major(metadata.rdev() as _) as _,
+            minor(metadata.rdev() as _) as _,
+        ];
+        let result = ts.ucmd().args(&["--printf", test_str, device]).succeeds();
+        let output = result
+            .stdout_str()
+            .split(' ')
+            .enumerate()
+            .map(parse)
+            .collect::<Result<Vec<i128>, _>>()
+            .unwrap();
+        assert_eq!(output, &expected);
+    }
+}