From 204c1edf9bb8494100151143cf5a0632ada5f2dc Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Sun, 19 Jul 2026 19:30:19 -0400 Subject: [PATCH] bug: fix incorrect bound length for comm parentheses searching (#2163) I made the assumption that the max length of the comm field is 16 bytes. This is wrong though - it's now 64. Note we may want to change it in the future to work regardless of potential length. --- CHANGELOG.md | 1 + src/collection/processes/linux/process.rs | 68 +++++++++++++++++++++-- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 49313001..7130e7ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ That said, these are more guidelines rather than hard rules, though the project ### Bug Fixes - [#2162](https://github.com/ClementTsang/bottom/pull/2162): Fix incorrect column headers missing shortcut hints in the processes widget. +- [#2163](https://github.com/ClementTsang/bottom/pull/2163): Fix incorrect bound length for comm parentheses searching. ## 0.14.5 - 2026-07-18 diff --git a/src/collection/processes/linux/process.rs b/src/collection/processes/linux/process.rs index 661f449c..73e5c1fb 100644 --- a/src/collection/processes/linux/process.rs +++ b/src/collection/processes/linux/process.rs @@ -90,18 +90,27 @@ impl Stat { let start_paren = line .find('(') .ok_or_else(|| anyhow!("start paren missing"))?; + // So, we _could_ parse the entire line from the end with rfind, but this is kinda inefficient, since we - // know the comm field is in the start. But, we know that he comm field is never more than 16 bytes + - // the start/end bracket characters, for a total of 18 bytes max. So we can bound our search! So starting - // from start_paren, just add 18 and rfind! + // know the comm field is in the start. But, we know that he comm field is never more than 64 bytes + + // the start/end bracket characters, for a total of 66 bytes max. So we can bound our search! So starting + // from start_paren, just add 66 and rfind! // - // Source: https://man.archlinux.org/man/proc_pid_stat.5.en - const TASK_COMM_LEN: usize = 16; - let end_paren = line[start_paren..start_paren + TASK_COMM_LEN + 2] + // Note that many online sources will say the max is 16 bytes - this is true for user processes, but kernel + // threads and work-queue threads are allowed to be longer. + // + // Sources: + // - https://man.archlinux.org/man/proc_pid_stat.5.en + // - https://stackoverflow.com/questions/23534263/what-is-the-maximum-allowed-limit-on-the-length-of-a-process-name#comment138697304_23534499 + // - https://elixir.bootlin.com/linux/v7.1.3/source/fs/proc/array.c#L100 + const MAX_COMM_LEN: usize = 64; + let end_search = line.len().min(start_paren + MAX_COMM_LEN + 2); + let end_paren = line[start_paren..end_search] .rfind(')') .ok_or_else(|| anyhow!("end paren missing"))? + start_paren; + // TODO: Maybe make this not panic. ( line[start_paren + 1..end_paren].to_string(), &line[end_paren + 2..], @@ -400,3 +409,50 @@ fn threads(root: &mut PathBuf, pid: Pid, get_threads: bool) -> Vec { Vec::new() } + +#[cfg(test)] +mod tests { + use std::io::{Seek, Write}; + + use super::*; + + fn stat_from_name(name: &str) -> anyhow::Result { + // Kinda garbage data but it should be fine. + let stat = format!( + "0 ({name}) R 0 0 0 0 -1 0 0 0 0 0 0 0 0 0 0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0" + ); + + let mut file = tempfile::tempfile().unwrap(); + file.write_all(stat.as_bytes()).unwrap(); + file.rewind().unwrap(); + + Stat::from_file(file, &mut String::new()) + } + + #[test] + fn parse_short_comm() { + let stat = stat_from_name("kworker/u16:2").unwrap(); + assert_eq!(stat.comm, "kworker/u16:2"); + } + + #[test] + fn parse_long_comm() { + let stat = stat_from_name("kworker/u16:2-events_unbound").unwrap(); + assert_eq!(stat.comm, "kworker/u16:2-events_unbound"); + } + + #[test] + fn parse_64_char_comm() { + let comm = "a".repeat(64); + let stat = stat_from_name(comm.as_str()).unwrap(); + + assert_eq!(stat.comm, comm); + } + + #[test] + fn parse_double_paren_comm() { + let stat = stat_from_name("(sd-pam)").unwrap(); + + assert_eq!(stat.comm, "(sd-pam)"); + } +}