pmap: fixed padding of Address, Offset, and Device fields (#458)

* pmap: fixed padding of Address, Offset, and Device fields

* pmap: fixed tests for more and most extended formats

Author: estodi

src/uu/pmap/src/maps_format_parser.rs

@@ -10,15 +10,37 @@ use std::io::{Error, ErrorKind};
 // Represents a parsed single line from /proc/<PID>/maps.
 #[derive(Debug, Clone, Default, PartialEq)]
 pub struct MapLine {
-    pub address: String,
+    pub address: Address,
     pub size_in_kb: u64,
     pub perms: Perms,
     pub offset: String,
-    pub device: String,
+    pub device: Device,
     pub inode: u64,
     pub mapping: String,
 }
 
+#[derive(Debug, Clone, Default, PartialEq)]
+pub struct Address {
+    pub start: String,
+    pub low: u64,
+    pub high: u64,
+}
+
+impl fmt::Display for Address {
+    // By default, pads with white spaces.
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{: >16}", self.start)
+    }
+}
+
+impl Address {
+    // Format for default, extended option, and device option.
+    // Pads the start address with zero.
+    pub fn zero_pad(&self) -> String {
+        format!("{:0>16}", self.start)
+    }
+}
+
 // Represents a set of permissions from the "perms" column of /proc/<PID>/maps.
 #[derive(Clone, Copy, Debug, Default, PartialEq)]
 pub struct Perms {
@@ -69,6 +91,28 @@ impl Perms {
     }
 }
 
+#[derive(Debug, Clone, Default, PartialEq)]
+pub struct Device {
+    pub major: String,
+    pub minor: String,
+    pub width: usize,
+}
+
+impl fmt::Display for Device {
+    // By default, does not pad.
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}:{}", self.major, self.minor)
+    }
+}
+
+impl Device {
+    // Format for device option.
+    // Pads the device info from /proc/<PID>/maps with zeros and turns AB:CD into 0AB:000CD.
+    pub fn device(&self) -> String {
+        format!("{:0>3}:{:0>5}", self.major, self.minor)
+    }
+}
+
 // Parses a single line from /proc/<PID>/maps. See
 // https://www.kernel.org/doc/html/latest/filesystems/proc.html for details about the expected
 // format.
@@ -90,7 +134,7 @@ pub fn parse_map_line(line: &str) -> Result<MapLine, Error> {
     let (offset, rest) = rest
         .split_once(' ')
         .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    let offset = format!("{offset:0>16}");
+    let offset = format!("{offset:0>8}");
 
     let (device, rest) = rest
         .split_once(' ')
@@ -116,9 +160,8 @@ pub fn parse_map_line(line: &str) -> Result<MapLine, Error> {
     })
 }
 
-// Returns the start address and the size of the provided memory range. The start address is always
-// 16-digits and padded with 0, if necessary. The size is in KB.
-fn parse_address(memory_range: &str) -> Result<(String, u64), Error> {
+// Returns Address instance and the size of the provided memory range. The size is in KB.
+fn parse_address(memory_range: &str) -> Result<(Address, u64), Error> {
     let (start, end) = memory_range
         .split_once('-')
         .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
@@ -127,15 +170,26 @@ fn parse_address(memory_range: &str) -> Result<(String, u64), Error> {
     let high = u64::from_str_radix(end, 16).map_err(|_| Error::from(ErrorKind::InvalidData))?;
     let size_in_kb = (high - low) / 1024;
 
-    Ok((format!("{start:0>16}"), size_in_kb))
+    Ok((
+        Address {
+            start: start.to_string(),
+            low,
+            high,
+        },
+        size_in_kb,
+    ))
 }
 
-// Pads the device info from /proc/<PID>/maps with zeros and turns AB:CD into 0AB:000CD.
-fn parse_device(device: &str) -> Result<String, Error> {
+// Returns Device instance.
+fn parse_device(device: &str) -> Result<Device, Error> {
     let (major, minor) = device
         .split_once(':')
         .ok_or_else(|| Error::from(ErrorKind::InvalidData))?;
-    Ok(format!("{major:0>3}:{minor:0>5}"))
+    Ok(Device {
+        major: major.to_string(),
+        minor: minor.to_string(),
+        width: device.len(),
+    })
 }
 
 impl MapLine {
@@ -174,19 +228,31 @@ mod test {
 
     fn create_map_line(
         address: &str,
+        low: u64,
+        high: u64,
         size_in_kb: u64,
         perms: Perms,
         offset: &str,
-        device: &str,
+        major: &str,
+        minor: &str,
+        width: usize,
         inode: u64,
         mapping: &str,
     ) -> MapLine {
         MapLine {
-            address: address.to_string(),
+            address: Address {
+                start: address.to_string(),
+                low,
+                high,
+            },
             size_in_kb,
             perms,
             offset: offset.to_string(),
-            device: device.to_string(),
+            device: Device {
+                major: major.to_string(),
+                minor: minor.to_string(),
+                width,
+            },
             inode,
             mapping: mapping.to_string(),
         }
@@ -210,31 +276,31 @@ mod test {
     fn test_parse_map_line() {
         let data = [
             (
-                create_map_line("000062442eb9e000", 16, Perms::from("r--p"), "0000000000000000", "008:00008", 10813151, "/usr/bin/konsole"),
+                create_map_line("62442eb9e000", 0x62442eb9e000, 0x62442eba2000, 16, Perms::from("r--p"), "00000000", "08", "08", 5, 10813151, "/usr/bin/konsole"),
                 "62442eb9e000-62442eba2000 r--p 00000000 08:08 10813151                   /usr/bin/konsole"
             ),
             (
-                create_map_line("000071af50000000", 132, Perms::from("rw-p"), "0000000000000000", "000:00000", 0, ""),
+                create_map_line("71af50000000", 0x71af50000000, 0x71af50021000,  132, Perms::from("rw-p"), "00000000", "00", "00", 5, 0, ""),
                 "71af50000000-71af50021000 rw-p 00000000 00:00 0 "
             ),
             (
-                create_map_line("00007ffc3f8df000", 132, Perms::from("rw-p"), "0000000000000000", "000:00000", 0, "[stack]"),
+                create_map_line("7ffc3f8df000", 0x7ffc3f8df000, 0x7ffc3f900000, 132, Perms::from("rw-p"), "00000000", "00", "00", 5, 0, "[stack]"),
                 "7ffc3f8df000-7ffc3f900000 rw-p 00000000 00:00 0                          [stack]"
             ),
             (
-                create_map_line("000071af8c9e6000", 16, Perms::from("rw-s"), "0000000105830000", "000:00010", 1075, "anon_inode:i915.gem"),
+                create_map_line("71af8c9e6000", 0x71af8c9e6000, 0x71af8c9ea000, 16, Perms::from("rw-s"), "105830000", "00", "10", 5, 1075, "anon_inode:i915.gem"),
                 "71af8c9e6000-71af8c9ea000 rw-s 105830000 00:10 1075                      anon_inode:i915.gem"
             ),
             (
-                create_map_line("000071af6cf0c000", 3560, Perms::from("rw-s"), "0000000000000000", "000:00001", 256481, "/memfd:wayland-shm (deleted)"),
+                create_map_line("71af6cf0c000", 0x71af6cf0c000, 0x71af6d286000, 3560, Perms::from("rw-s"), "00000000", "00", "01", 5, 256481, "/memfd:wayland-shm (deleted)"),
                 "71af6cf0c000-71af6d286000 rw-s 00000000 00:01 256481                     /memfd:wayland-shm (deleted)"
             ),
             (
-                create_map_line("ffffffffff600000", 4, Perms::from("--xp"), "0000000000000000", "000:00000", 0, "[vsyscall]"),
+                create_map_line("ffffffffff600000", 0xffffffffff600000, 0xffffffffff601000, 4, Perms::from("--xp"), "00000000", "00", "00", 5, 0, "[vsyscall]"),
                 "ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0                  [vsyscall]"
             ),
             (
-                create_map_line("00005e8187da8000", 24, Perms::from("r--p"), "0000000000000000", "008:00008", 9524160, "/usr/bin/hello   world"),
+                create_map_line("5e8187da8000", 0x5e8187da8000, 0x5e8187dae000, 24, Perms::from("r--p"), "00000000", "08", "08", 5, 9524160, "/usr/bin/hello   world"),
                 "5e8187da8000-5e8187dae000 r--p 00000000 08:08 9524160                    /usr/bin/hello   world"
             ),
         ];
@@ -251,12 +317,16 @@ mod test {
 
     #[test]
     fn test_parse_address() {
-        let (start, size) = parse_address("ffffffffff600000-ffffffffff601000").unwrap();
-        assert_eq!(start, "ffffffffff600000");
+        let (address, size) = parse_address("ffffffffff600000-ffffffffff601000").unwrap();
+        assert_eq!(address.start, "ffffffffff600000");
+        assert_eq!(address.low, 0xffffffffff600000);
+        assert_eq!(address.high, 0xffffffffff601000);
         assert_eq!(size, 4);
 
-        let (start, size) = parse_address("7ffc4f0c2000-7ffc4f0e3000").unwrap();
-        assert_eq!(start, "00007ffc4f0c2000");
+        let (address, size) = parse_address("7ffc4f0c2000-7ffc4f0e3000").unwrap();
+        assert_eq!(address.start, "7ffc4f0c2000");
+        assert_eq!(address.low, 0x7ffc4f0c2000);
+        assert_eq!(address.high, 0x7ffc4f0e3000);
         assert_eq!(size, 132);
     }
 
@@ -273,8 +343,15 @@ mod test {
 
     #[test]
     fn test_parse_device() {
-        assert_eq!("012:00034", parse_device("12:34").unwrap());
-        assert_eq!("000:00000", parse_device("00:00").unwrap());
+        assert_eq!("12:34", parse_device("12:34").unwrap().to_string());
+        assert_eq!("00:00", parse_device("00:00").unwrap().to_string());
+        assert_eq!("fe:01", parse_device("fe:01").unwrap().to_string());
+        assert_eq!("103:100", parse_device("103:100").unwrap().to_string());
+
+        assert_eq!("012:00034", parse_device("12:34").unwrap().device());
+        assert_eq!("000:00000", parse_device("00:00").unwrap().device());
+        assert_eq!("0fe:00001", parse_device("fe:01").unwrap().device());
+        assert_eq!("103:00100", parse_device("103:100").unwrap().device());
     }
 
     #[test]

src/uu/pmap/src/pmap.rs

@@ -184,7 +184,7 @@ fn output_default_format(pid: &str, pmap_config: &PmapConfig) -> Result<(), Erro
     process_maps(pid, None, |map_line| {
         println!(
             "{} {:>6}K {} {}",
-            map_line.address,
+            map_line.address.zero_pad(),
             map_line.size_in_kb,
             map_line.perms.mode(),
             map_line.parse_mapping(pmap_config)
@@ -209,7 +209,7 @@ fn output_extended_format(pid: &str, pmap_config: &PmapConfig) -> Result<(), Err
     for smap_entry in smap_table.entries {
         println!(
             "{} {:>7} {:>7} {:>7} {} {}",
-            smap_entry.map_line.address,
+            smap_entry.map_line.address.zero_pad(),
             smap_entry.map_line.size_in_kb,
             smap_entry.rss_in_kb,
             smap_entry.shared_dirty_in_kb + smap_entry.private_dirty_in_kb,
@@ -361,12 +361,12 @@ fn output_device_format(pid: &str, pmap_config: &PmapConfig) -> Result<(), Error
         },
         |map_line| {
             println!(
-                "{} {:>7} {} {} {} {}",
-                map_line.address,
+                "{} {:>7} {} {:0>16} {} {}",
+                map_line.address.zero_pad(),
                 map_line.size_in_kb,
                 map_line.perms.mode(),
                 map_line.offset,
-                map_line.device,
+                map_line.device.device(),
                 map_line.parse_mapping(pmap_config)
             );
             total_mapped += map_line.size_in_kb;