Repository: uutils/procps (rust)

Issue:
pmap: don't output header if `/proc/<PID>/smaps` can't be accessed
While reviewing https://github.com/uutils/procps/pull/368 I noticed that we output a header if the permission is denied to read `/proc/<PID>/smaps` whereas the original `pmap` doesn't show a header. On my machine, this seems only to affect PID 1.
```
$ cargo run -q pmap -x 1
1:   /sbin/init \vmlinuz-linux
Address           Kbytes     RSS   Dirty Mode  Mapping
$ pmap -x 1
1:   /sbin/init \vmlinuz-linux
```

Pull request:
pmap: fixed to hide the header when permission is denied
Resolves #376 

As the issue is not limited to `smaps`, several parts of the code are modified.

Patch:
diff --git a/src/uu/pmap/src/pmap.rs b/src/uu/pmap/src/pmap.rs
index cc5443c..9617304 100644
--- a/src/uu/pmap/src/pmap.rs
+++ b/src/uu/pmap/src/pmap.rs
@@ -81,13 +81,17 @@ fn parse_cmdline(pid: &str) -> Result<String, Error> {
     Ok(cmdline.into())
 }
 
-fn process_maps<F>(pid: &str, mut process_line: F) -> Result<(), Error>
+fn process_maps<F>(pid: &str, header: Option<&str>, mut process_line: F) -> Result<(), Error>
 where
     F: FnMut(&MapLine),
 {
     let path = format!("/proc/{pid}/maps");
     let contents = fs::read_to_string(path)?;
 
+    if let Some(header) = header {
+        println!("{header}");
+    }
+
     for line in contents.lines() {
         let map_line = parse_map_line(line)?;
         process_line(&map_line);
@@ -96,7 +100,7 @@ where
     Ok(())
 }
 
-fn process_smaps<F>(pid: &str, mut process_entry: F) -> Result<(), Error>
+fn process_smaps<F>(pid: &str, header: Option<&str>, mut process_entry: F) -> Result<(), Error>
 where
     F: FnMut(&SmapEntry),
 {
@@ -104,6 +108,10 @@ where
     let contents = fs::read_to_string(path)?;
     let smap_entries = parse_smap_entries(&contents)?;
 
+    if let Some(header) = header {
+        println!("{header}");
+    }
+
     for entry in smap_entries {
         process_entry(&entry);
     }
@@ -114,7 +122,7 @@ where
 fn output_default_format(pid: &str) -> Result<(), Error> {
     let mut total = 0;
 
-    process_maps(pid, |map_line| {
+    process_maps(pid, None, |map_line| {
         println!(
             "{} {:>6}K {} {}",
             map_line.address, map_line.size_in_kb, map_line.perms, map_line.mapping
@@ -132,22 +140,24 @@ fn output_extended_format(pid: &str) -> Result<(), Error> {
     let mut total_rss = 0;
     let mut total_dirty = 0;
 
-    println!("Address           Kbytes     RSS   Dirty Mode  Mapping");
-
-    process_smaps(pid, |smap_entry| {
-        println!(
-            "{} {:>7} {:>7} {:>7} {} {}",
-            smap_entry.map_line.address,
-            smap_entry.map_line.size_in_kb,
-            smap_entry.rss_in_kb,
-            smap_entry.shared_dirty_in_kb + smap_entry.private_dirty_in_kb,
-            smap_entry.map_line.perms,
-            smap_entry.map_line.mapping
-        );
-        total_mapped += smap_entry.map_line.size_in_kb;
-        total_rss += smap_entry.rss_in_kb;
-        total_dirty += smap_entry.shared_dirty_in_kb + smap_entry.private_dirty_in_kb;
-    })?;
+    process_smaps(
+        pid,
+        Some("Address           Kbytes     RSS   Dirty Mode  Mapping"),
+        |smap_entry| {
+            println!(
+                "{} {:>7} {:>7} {:>7} {} {}",
+                smap_entry.map_line.address,
+                smap_entry.map_line.size_in_kb,
+                smap_entry.rss_in_kb,
+                smap_entry.shared_dirty_in_kb + smap_entry.private_dirty_in_kb,
+                smap_entry.map_line.perms,
+                smap_entry.map_line.mapping
+            );
+            total_mapped += smap_entry.map_line.size_in_kb;
+            total_rss += smap_entry.rss_in_kb;
+            total_dirty += smap_entry.shared_dirty_in_kb + smap_entry.private_dirty_in_kb;
+        },
+    )?;
 
     println!("---------------- ------- ------- ------- ");
     println!("total kB         {total_mapped:>7} {total_rss:>7} {total_dirty:>7}");
@@ -160,28 +170,30 @@ fn output_device_format(pid: &str) -> Result<(), Error> {
     let mut total_writeable_private = 0;
     let mut total_shared = 0;
 
-    println!("Address           Kbytes Mode  Offset           Device    Mapping");
-
-    process_maps(pid, |map_line| {
-        println!(
-            "{} {:>7} {} {} {} {}",
-            map_line.address,
-            map_line.size_in_kb,
-            map_line.perms,
-            map_line.offset,
-            map_line.device,
-            map_line.mapping
-        );
-        total_mapped += map_line.size_in_kb;
-
-        if map_line.perms.writable && !map_line.perms.shared {
-            total_writeable_private += map_line.size_in_kb;
-        }
+    process_maps(
+        pid,
+        Some("Address           Kbytes Mode  Offset           Device    Mapping"),
+        |map_line| {
+            println!(
+                "{} {:>7} {} {} {} {}",
+                map_line.address,
+                map_line.size_in_kb,
+                map_line.perms,
+                map_line.offset,
+                map_line.device,
+                map_line.mapping
+            );
+            total_mapped += map_line.size_in_kb;
+
+            if map_line.perms.writable && !map_line.perms.shared {
+                total_writeable_private += map_line.size_in_kb;
+            }
 
-        if map_line.perms.shared {
-            total_shared += map_line.size_in_kb;
-        }
-    })?;
+            if map_line.perms.shared {
+                total_shared += map_line.size_in_kb;
+            }
+        },
+    )?;
 
     println!(
         "mapped: {total_mapped}K    writeable/private: {total_writeable_private}K    shared: {total_shared}K"
diff --git a/src/uu/w/src/w.rs b/src/uu/w/src/w.rs
index f1d803f..852b6b0 100644
--- a/src/uu/w/src/w.rs
+++ b/src/uu/w/src/w.rs
@@ -204,7 +204,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                     println!("{:<9}{:<9}{:<7}{:<}", "USER", "TTY", "IDLE", "WHAT");
                 } else {
                     println!(
-                        "{:<9}{:<9}{:<9}{:<6} {:<7}{:<5}{:<}",
+                        "{:<9}{:<10}{:<9}{:<6} {:<7}{:<6}{:<}",
                         "USER", "TTY", "LOGIN@", "IDLE", "JCPU", "PCPU", "WHAT"
                     );
                 }
@@ -220,7 +220,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
                     );
                 } else {
                     println!(
-                        "{:<9}{:<9}{:<9}{:<6} {:<7}{:<5}{:<}",
+                        "{:<9}{:<10}{:<9}{:<6} {:<7}{:<6}{:<}",
                         user.user,
                         user.terminal,
                         user.login_time,


Test patch:
diff --git a/tests/by-util/test_pmap.rs b/tests/by-util/test_pmap.rs
index 5c196a0..032cb99 100644
--- a/tests/by-util/test_pmap.rs
+++ b/tests/by-util/test_pmap.rs
@@ -10,6 +10,8 @@ use regex::Regex;
 use std::process;
 
 const NON_EXISTING_PID: &str = "999999";
+#[cfg(target_os = "linux")]
+const INIT_PID: &str = "1";
 
 #[test]
 fn test_no_args() {
@@ -73,6 +75,20 @@ fn test_non_existing_pid() {
         .no_output();
 }
 
+#[test]
+#[cfg(target_os = "linux")]
+fn test_permission_denied() {
+    let result = new_ucmd!()
+        .arg(INIT_PID)
+        .fails()
+        .code_is(1)
+        .no_stderr()
+        .clone()
+        .stdout_move_str();
+
+    assert_cmdline_only(INIT_PID, &result);
+}
+
 #[test]
 #[cfg(target_os = "linux")]
 fn test_extended() {
@@ -89,6 +105,23 @@ fn test_extended() {
     }
 }
 
+#[test]
+#[cfg(target_os = "linux")]
+fn test_extended_permission_denied() {
+    for arg in ["-x", "--extended"] {
+        let result = new_ucmd!()
+            .arg(arg)
+            .arg(INIT_PID)
+            .fails()
+            .code_is(1)
+            .no_stderr()
+            .clone()
+            .stdout_move_str();
+
+        assert_cmdline_only(INIT_PID, &result);
+    }
+}
+
 #[test]
 #[cfg(target_os = "linux")]
 fn test_device() {
@@ -105,11 +138,38 @@ fn test_device() {
     }
 }
 
+#[test]
+#[cfg(target_os = "linux")]
+fn test_device_permission_denied() {
+    for arg in ["-d", "--device"] {
+        let result = new_ucmd!()
+            .arg(arg)
+            .arg(INIT_PID)
+            .fails()
+            .code_is(1)
+            .no_stderr()
+            .clone()
+            .stdout_move_str();
+
+        assert_cmdline_only(INIT_PID, &result);
+    }
+}
+
 #[test]
 fn test_invalid_arg() {
     new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
 }
 
+// Ensure `s` has the following format:
+//
+// 1234:   /some/path
+#[cfg(target_os = "linux")]
+fn assert_cmdline_only(pid: &str, s: &str) {
+    let s = s.trim_end();
+    let re = Regex::new(&format!("^{pid}:   .+[^ ]$")).unwrap();
+    assert!(re.is_match(s));
+}
+
 // Ensure `s` has the following format:
 //
 // 1234:   /some/path