mirror of
https://github.com/ClementTsang/bottom.git
synced 2026-07-27 23:01:31 +00:00
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.
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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<PathBuf> {
|
||||
|
||||
Vec::new()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::io::{Seek, Write};
|
||||
|
||||
use super::*;
|
||||
|
||||
fn stat_from_name(name: &str) -> anyhow::Result<Stat> {
|
||||
// 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)");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user