Merge pull request #578 from dezgeg/tty_fix

process: Fix tty resolving

Author: cakebaker

src/uu/pgrep/src/process.rs

@@ -6,6 +6,8 @@
 use regex::Regex;
 use std::fs::read_link;
 use std::hash::Hash;
+#[cfg(target_os = "linux")]
+use std::ops::RangeInclusive;
 use std::sync::{LazyLock, OnceLock};
 use std::{
     collections::HashMap,
@@ -15,20 +17,130 @@ use std::{
 };
 use walkdir::{DirEntry, WalkDir};
 
+/// Represents a TTY driver entry from /proc/tty/drivers
+#[cfg(target_os = "linux")]
+#[derive(Debug, Clone, PartialEq, Eq)]
+struct TtyDriverEntry {
+    device_prefix: String,
+    major: u32,
+    minor_range: RangeInclusive<u32>,
+}
+
+#[cfg(target_os = "linux")]
+impl TtyDriverEntry {
+    fn new(device_prefix: String, major: u32, minor_range: RangeInclusive<u32>) -> Self {
+        Self {
+            device_prefix,
+            major,
+            minor_range,
+        }
+    }
+
+    fn device_path_if_matches(&self, major: u32, minor: u32) -> Option<String> {
+        if self.major != major || !self.minor_range.contains(&minor) {
+            return None;
+        }
+
+        // /dev/pts devices are in a subdirectory unlike others
+        if self.device_prefix == "/dev/pts" {
+            return Some(format!("/dev/pts/{}", minor));
+        }
+
+        // If there is only one minor (e.g. /dev/console) it should not get a number
+        if self.minor_range.start() == self.minor_range.end() {
+            Some(self.device_prefix.clone())
+        } else {
+            let device_number = minor - self.minor_range.start();
+            Some(format!("{}{}", self.device_prefix, device_number))
+        }
+    }
+}
+
+#[cfg(target_os = "linux")]
+static TTY_DRIVERS_CACHE: LazyLock<Vec<TtyDriverEntry>> = LazyLock::new(|| {
+    fs::read_to_string("/proc/tty/drivers")
+        .map(|content| parse_proc_tty_drivers(&content))
+        .unwrap_or_default()
+});
+
+#[cfg(target_os = "linux")]
+fn parse_proc_tty_drivers(drivers_content: &str) -> Vec<TtyDriverEntry> {
+    // Example lines:
+    // /dev/tty             /dev/tty        5       0 system:/dev/tty
+    // /dev/vc/0            /dev/vc/0       4       0 system:vtmaster
+    // hvc                  /dev/hvc      229 0-7 system
+    // serial               /dev/ttyS       4 64-95 serial
+    // pty_slave            /dev/pts      136 0-1048575 pty:slave
+    let regex = Regex::new(r"^[^ ]+ +([^ ]+) +(\d+) +(\d+)(?:-(\d+))?").unwrap();
+
+    let mut entries = Vec::new();
+    for line in drivers_content.lines() {
+        let Some(captures) = regex.captures(line) else {
+            continue;
+        };
+
+        let device_prefix = captures[1].to_string();
+        let Ok(major) = captures[2].parse::<u32>() else {
+            continue;
+        };
+        let Ok(min_minor) = captures[3].parse::<u32>() else {
+            continue;
+        };
+        let max_minor = captures
+            .get(4)
+            .and_then(|m| m.as_str().parse::<u32>().ok())
+            .unwrap_or(min_minor);
+
+        entries.push(TtyDriverEntry::new(
+            device_prefix,
+            major,
+            min_minor..=max_minor,
+        ));
+    }
+
+    entries
+}
+
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum Teletype {
-    Tty(u64),
-    TtyS(u64),
-    Pts(u64),
+    Known(String),
     Unknown,
 }
 
+impl Teletype {
+    #[cfg(target_os = "linux")]
+    pub fn from_tty_nr(tty_nr: u64) -> Self {
+        Self::from_tty_nr_impl(tty_nr, &TTY_DRIVERS_CACHE)
+    }
+
+    #[cfg(not(target_os = "linux"))]
+    pub fn from_tty_nr(_tty_nr: u64) -> Self {
+        Self::Unknown
+    }
+
+    #[cfg(target_os = "linux")]
+    fn from_tty_nr_impl(tty_nr: u64, drivers: &[TtyDriverEntry]) -> Self {
+        use uucore::libc::{major, minor};
+
+        if tty_nr == 0 {
+            return Self::Unknown;
+        }
+
+        let (major_dev, minor_dev) = (major(tty_nr), minor(tty_nr));
+        for entry in drivers.iter() {
+            if let Some(device_path) = entry.device_path_if_matches(major_dev, minor_dev) {
+                return Self::Known(device_path);
+            }
+        }
+
+        Self::Unknown
+    }
+}
+
 impl Display for Teletype {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         match self {
-            Self::Tty(id) => write!(f, "/dev/pts/{id}"),
-            Self::TtyS(id) => write!(f, "/dev/tty{id}"),
-            Self::Pts(id) => write!(f, "/dev/ttyS{id}"),
+            Self::Known(device_path) => write!(f, "{}", device_path),
             Self::Unknown => write!(f, "?"),
         }
     }
@@ -58,43 +170,8 @@ impl TryFrom<PathBuf> for Teletype {
     type Error = ();
 
     fn try_from(value: PathBuf) -> Result<Self, Self::Error> {
-        // Three case: /dev/pts/* , /dev/ttyS**, /dev/tty**
-
-        let mut iter = value.iter();
-        // Case 1
-
-        // Considering this format: **/**/pts/<num>
-        if let (Some(_), Some(num)) = (iter.find(|it| *it == "pts"), iter.next()) {
-            return num
-                .to_str()
-                .ok_or(())?
-                .parse::<u64>()
-                .map_err(|_| ())
-                .map(Teletype::Pts);
-        };
-
-        // Considering this format: **/**/ttyS** then **/**/tty**
-        let path = value.to_str().ok_or(())?;
-
-        let f = |prefix: &str| {
-            value
-                .iter()
-                .next_back()?
-                .to_str()?
-                .strip_prefix(prefix)?
-                .parse::<u64>()
-                .ok()
-        };
-
-        if path.contains("ttyS") {
-            // Case 2
-            f("ttyS").ok_or(()).map(Teletype::TtyS)
-        } else if path.contains("tty") {
-            // Case 3
-            f("tty").ok_or(()).map(Teletype::Tty)
-        } else {
-            Err(())
-        }
+        let path_str = value.to_str().ok_or(())?;
+        Ok(Self::Known(path_str.to_string()))
     }
 }
 
@@ -602,28 +679,17 @@ impl ProcessInformation {
         RunState::try_from(self.stat().get(2).unwrap().as_str())
     }
 
-    /// This function will scan the `/proc/<pid>/fd` directory
-    ///
-    /// If the process does not belong to any terminal and mismatched permission,
-    /// the result will contain [TerminalType::Unknown].
+    /// Get the controlling terminal from the tty_nr field in /proc/<pid>/stat
     ///
-    /// Otherwise [TerminalType::Unknown] does not appear in the result.
-    pub fn tty(&self) -> Teletype {
-        let path = PathBuf::from(format!("/proc/{}/fd", self.pid));
-
-        let Ok(result) = fs::read_dir(path) else {
-            return Teletype::Unknown;
+    /// Returns Teletype::Unknown if the process has no controlling terminal (tty_nr == 0)
+    /// or if the tty_nr cannot be resolved to a device.
+    pub fn tty(&mut self) -> Teletype {
+        let tty_nr = match self.get_numeric_stat_field(6) {
+            Ok(tty_nr) => tty_nr,
+            Err(_) => return Teletype::Unknown,
         };
 
-        for dir in result.flatten().filter(|it| it.path().is_symlink()) {
-            if let Ok(path) = fs::read_link(dir.path()) {
-                if let Ok(tty) = Teletype::try_from(path) {
-                    return tty;
-                }
-            }
-        }
-
-        Teletype::Unknown
+        Teletype::from_tty_nr(tty_nr)
     }
 
     pub fn thread_ids(&mut self) -> &[usize] {
@@ -734,6 +800,53 @@ mod tests {
     #[cfg(target_os = "linux")]
     use uucore::process::getpid;
 
+    #[test]
+    #[cfg(target_os = "linux")]
+    fn test_tty_resolution() {
+        let test_content = r#"/dev/tty             /dev/tty        5       0 system:/dev/tty
+/dev/console         /dev/console    5       1 system:console
+/dev/ptmx            /dev/ptmx       5       2 system
+/dev/vc/0            /dev/vc/0       4       0 system:vtmaster
+hvc                  /dev/hvc      229 0-7 system
+serial               /dev/ttyS       4 64-95 serial
+pty_slave            /dev/pts      136 0-1048575 pty:slave
+pty_master           /dev/ptm      128 0-1048575 pty:master
+unknown              /dev/tty        4 1-63 console"#;
+
+        let expected_entries = vec![
+            TtyDriverEntry::new("/dev/tty".to_string(), 5, 0..=0),
+            TtyDriverEntry::new("/dev/console".to_string(), 5, 1..=1),
+            TtyDriverEntry::new("/dev/ptmx".to_string(), 5, 2..=2),
+            TtyDriverEntry::new("/dev/vc/0".to_string(), 4, 0..=0),
+            TtyDriverEntry::new("/dev/hvc".to_string(), 229, 0..=7),
+            TtyDriverEntry::new("/dev/ttyS".to_string(), 4, 64..=95),
+            TtyDriverEntry::new("/dev/pts".to_string(), 136, 0..=1048575),
+            TtyDriverEntry::new("/dev/ptm".to_string(), 128, 0..=1048575),
+            TtyDriverEntry::new("/dev/tty".to_string(), 4, 1..=63),
+        ];
+
+        let parsed_entries = parse_proc_tty_drivers(test_content);
+        assert_eq!(parsed_entries, expected_entries);
+
+        let test_cases = vec![
+            // (major, minor, expected_result)
+            (0, 0, Teletype::Unknown),
+            (5, 0, Teletype::Known("/dev/tty".to_string())),
+            (5, 1, Teletype::Known("/dev/console".to_string())),
+            (136, 123, Teletype::Known("/dev/pts/123".to_string())),
+            (4, 64, Teletype::Known("/dev/ttyS0".to_string())),
+            (4, 65, Teletype::Known("/dev/ttyS1".to_string())),
+            (229, 3, Teletype::Known("/dev/hvc3".to_string())),
+            (999, 999, Teletype::Unknown),
+        ];
+
+        for (major, minor, expected) in test_cases {
+            let tty_nr = uucore::libc::makedev(major, minor);
+            let result = Teletype::from_tty_nr_impl(tty_nr, &parsed_entries);
+            assert_eq!(result, expected);
+        }
+    }
+
     #[test]
     fn test_run_state_conversion() {
         assert_eq!(RunState::try_from("R").unwrap(), RunState::Running);
@@ -763,7 +876,14 @@ mod tests {
     #[test]
     #[cfg(target_os = "linux")]
     fn test_pid_entry() {
-        let pid_entry = ProcessInformation::current_process_info().unwrap();
+        use std::io::IsTerminal;
+
+        let mut pid_entry = ProcessInformation::current_process_info().unwrap();
+
+        if !std::io::stdout().is_terminal() && !std::io::stderr().is_terminal() {
+            assert_eq!(pid_entry.tty(), Teletype::Unknown);
+            return;
+        }
         let mut result = WalkDir::new(format!("/proc/{}/fd", getpid()))
             .into_iter()
             .flatten()

src/uu/ps/src/picker.rs

@@ -135,10 +135,11 @@ fn sid(proc_info: RefCell<ProcessInformation>) -> String {
 }
 
 fn tty(proc_info: RefCell<ProcessInformation>) -> String {
-    match proc_info.borrow().tty() {
-        Teletype::Tty(tty) => format!("tty{tty}"),
-        Teletype::TtyS(ttys) => format!("ttyS{ttys}"),
-        Teletype::Pts(pts) => format!("pts/{pts}"),
+    match proc_info.borrow_mut().tty() {
+        Teletype::Known(device_path) => device_path
+            .strip_prefix("/dev/")
+            .unwrap_or(&device_path)
+            .to_owned(),
         Teletype::Unknown => "?".to_owned(),
     }
 }

src/uu/snice/src/snice.rs

@@ -129,7 +129,7 @@ pub fn ask_user(pid: u32) -> bool {
     let process = process_snapshot().process(Pid::from_u32(pid)).unwrap();
 
     let tty = ProcessInformation::try_new(PathBuf::from_str(&format!("/proc/{pid}")).unwrap())
-        .map(|v| v.tty().to_string())
+        .map(|mut v| v.tty().to_string())
         .unwrap_or(String::from("?"));
 
     let user = process
@@ -188,7 +188,8 @@ pub fn construct_verbose_result(
             let process = process_snapshot().process(Pid::from_u32(pid)).unwrap();
 
             let tty =
-                ProcessInformation::try_new(PathBuf::from_str(&format!("/proc/{pid}")).unwrap());
+                ProcessInformation::try_new(PathBuf::from_str(&format!("/proc/{pid}")).unwrap())
+                    .map(|mut v| v.tty().to_string());
 
             let user = process
                 .user_id()
@@ -214,7 +215,7 @@ pub fn construct_verbose_result(
                 row![pid]
             }
             Some((tty, user, cmd, action)) => {
-                row![tty.unwrap().tty(), user, pid, cmd, action]
+                row![tty.unwrap(), user, pid, cmd, action]
             }
         })
         .collect::<Table>();