tyler.durden/bottom
1
0
Fork 0
mirror of https://github.com/ClementTsang/bottom.git synced 2025-09-21 08:47:53 +02:00
Code Issues Packages Projects Releases Wiki Activity

bug: fix colon at end of process name for now on Linux (#1800)

Browse Source 
* driveby use rustix

* refactor some code aeround

* bug: fix colon at end of process name for now

* clippy

* comments

* changelog

* some other changes + test

* extra test
This commit is contained in:
Clement Tsang 2025-08-21 07:21:44 -04:00 committed by GitHub
parent 6409f67dbc
commit 47cc0b346a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 117 additions and 64 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
CHANGELOG.md
View File

@ -26,6 +26,10 @@ That said, these are more guidelines rather than hardset rules, though the proje
- [#1793](https://github.com/ClementTsang/bottom/pull/1793): Add support for threads in Linux.
### Bug Fixes
- [#1800](https://github.com/ClementTsang/bottom/pull/1800): Fix colon at end of process name in Linux.
## [0.11.1] - 2025-08-15
### Bug Fixes

90
src/collection/processes/linux/mod.rs
View File

@ -137,7 +137,7 @@ fn read_proc(
thread_parent: Option<Pid>,
) -> CollectionResult<(ProcessHarvest, u64)> {
let Process {
pid: _,
pid: _pid,
uid,
stat,
io,
@ -221,39 +221,56 @@ fn read_proc(
};
let (command, name) = {
let truncated_name = stat.comm;
let comm = stat.comm;
if let Some(cmdline) = cmdline {
if cmdline.is_empty() {
(concat_string!("[", truncated_name, "]"), truncated_name)
(concat_string!("[", comm, "]"), comm)
} else {
let name = if truncated_name.len() >= MAX_STAT_NAME_LEN {
let first_part = match cmdline.split_once(' ') {
Some((first, _)) => first,
None => &cmdline,
};
// If the comm fits then we'll default to whatever is set.
// If it doesn't, we need to do some magic to determine what it's
// supposed to be.
//
// We follow something similar to how htop does it to identify a valid name based on the cmdline.
// - https://github.com/htop-dev/htop/blob/bcb18ef82269c68d54a160290e5f8b2e939674ec/Process.c#L268 (kinda)
// - https://github.com/htop-dev/htop/blob/bcb18ef82269c68d54a160290e5f8b2e939674ec/Process.c#L573
//
// Also note that cmdline is (for us) separated by \0.
// We're only interested in the executable part, not the file path (part of command),
// so strip everything but the command name if needed.
let command = match first_part.rsplit_once('/') {
Some((_, last)) => last,
None => first_part,
};
// TODO: Needed as some processes have stuff like "systemd-userwork: waiting..."
// command.trim_end_matches(':').to_string()
command.to_string()
// TODO: We might want to re-evaluate if we want to do it like this,
// as it turns out I was dumb and sometimes comm != process name...
//
// What we should do is store:
// - basename (what we're kinda doing now, except we're gating on comm length)
// - command (full thing)
// - comm (as a separate thing)
//
// Stuff like htop also offers the option to "highlight" basename and comm in command. Might be neat?
let name = if comm.len() >= MAX_STAT_NAME_LEN {
name_from_cmdline(&cmdline)
} else {
truncated_name
comm
};
(cmdline, name)
}
} else {
(truncated_name.clone(), truncated_name)
(comm.clone(), comm)
}
};
// We have moved command processing here.
// SAFETY: We are only replacing a single char (NUL) with another single char (space).
let mut command = command;
let buf_mut = unsafe { command.as_mut_vec() };
for byte in buf_mut {
if *byte == 0 {
const SPACE: u8 = ' '.to_ascii_lowercase() as u8;
*byte = SPACE;
}
}
Ok((
ProcessHarvest {
pid: process.pid,
@ -284,6 +301,22 @@ fn read_proc(
))
}
fn name_from_cmdline(cmdline: &str) -> String {
let mut start = 0;
let mut end = cmdline.len();
for (i, c) in cmdline.chars().enumerate() {
if c == '/' {
start = i + 1;
} else if c == '\0' || c == ':' {
end = i;
break;
}
}
cmdline[start..end].to_string()
}
pub(crate) struct PrevProc<'a> {
pub prev_idle: &'a mut f64,
pub prev_non_idle: &'a mut f64,
@ -502,4 +535,19 @@ mod tests {
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 10 values"
);
}
#[test]
fn test_name_from_cmdline() {
assert_eq!(name_from_cmdline("/usr/bin/btm"), "btm");
assert_eq!(name_from_cmdline("/usr/bin/btm\0--asdf\0--asdf/gkj"), "btm");
assert_eq!(name_from_cmdline("/usr/bin/btm:"), "btm");
assert_eq!(name_from_cmdline("/usr/bin/b tm"), "b tm");
assert_eq!(name_from_cmdline("/usr/bin/b tm:"), "b tm");
assert_eq!(name_from_cmdline("/usr/bin/b tm\0--test"), "b tm");
assert_eq!(name_from_cmdline("/usr/bin/b tm:\0--test"), "b tm");
assert_eq!(
name_from_cmdline("/usr/bin/b t m:\0--\"test thing\""),
"b t m"
);
}
}

87
src/collection/processes/linux/process.rs
View File

@ -237,7 +237,7 @@ impl Process {
) -> anyhow::Result<(Process, Vec<PathBuf>)> {
buffer.clear();
let fd = rustix::fs::openat(
let pid_dir = rustix::fs::openat(
rustix::fs::CWD,
pid_path.as_path(),
OFlags::PATH | OFlags::DIRECTORY | OFlags::CLOEXEC,
@ -257,7 +257,7 @@ impl Process {
.ok_or_else(|| anyhow!("PID for {pid_path:?} was not found"))?;
let uid = {
let metadata = rustix::fs::fstat(&fd);
let metadata = rustix::fs::fstat(&pid_dir);
match metadata {
Ok(md) => Some(md.st_uid),
Err(_) => None,
@ -271,10 +271,10 @@ impl Process {
// Stat is pretty long, do this first to pre-allocate up-front.
let stat =
open_at(&mut root, "stat", &fd).and_then(|file| Stat::from_file(file, buffer))?;
open_at(&mut root, "stat", &pid_dir).and_then(|file| Stat::from_file(file, buffer))?;
reset(&mut root, buffer);
let cmdline = if cmdline(&mut root, &fd, buffer).is_ok() {
let cmdline = if cmdline(&mut root, &pid_dir, buffer).is_ok() {
// The clone will give a string with the capacity of the length of buffer, don't worry.
Some(buffer.clone())
} else {
@ -282,37 +282,13 @@ impl Process {
};
reset(&mut root, buffer);
let io = open_at(&mut root, "io", &fd)
let io = open_at(&mut root, "io", &pid_dir)
.and_then(|file| Io::from_file(file, buffer))
.ok();
reset(&mut root, buffer);
let threads = if get_threads {
root.push("task");
if let Ok(task) = std::fs::read_dir(root) {
let pid_str = pid.to_string();
task.flatten()
.filter_map(|thread_dir| {
let file_name = thread_dir.file_name();
let file_name = file_name.to_string_lossy();
let file_name = file_name.trim();
if is_str_numeric(file_name) && file_name != pid_str {
Some(thread_dir.path())
} else {
None
}
})
.collect::<Vec<_>>()
} else {
Vec::new()
}
} else {
Vec::new()
};
let threads = threads(&mut root, pid, get_threads);
Ok((
Process {
@ -329,19 +305,7 @@ impl Process {
#[inline]
fn cmdline(root: &mut PathBuf, fd: &OwnedFd, buffer: &mut String) -> anyhow::Result<()> {
let _ = open_at(root, "cmdline", fd)
.map(|mut file| file.read_to_string(buffer))
.inspect(|_| {
// SAFETY: We are only replacing a single char (NUL) with another single char (space).
let buf_mut = unsafe { buffer.as_mut_vec() };
for byte in buf_mut {
if *byte == 0 {
const SPACE: u8 = ' '.to_ascii_lowercase() as u8;
*byte = SPACE;
}
}
})?;
let _ = open_at(root, "cmdline", fd).map(|mut file| file.read_to_string(buffer))?;
Ok(())
}
@ -356,3 +320,40 @@ fn open_at(root: &mut PathBuf, child: &str, fd: &OwnedFd) -> anyhow::Result<File
Ok(File::from(new_fd))
}
#[inline]
fn threads(root: &mut PathBuf, pid: Pid, get_threads: bool) -> Vec<PathBuf> {
if get_threads {
root.push("task");
let Ok(task_dir) = rustix::fs::openat(
rustix::fs::CWD,
root.as_path(),
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
Mode::empty(),
) else {
return Vec::new();
};
if let Ok(task) = rustix::fs::Dir::read_from(task_dir) {
let pid_str = pid.to_string();
return task
.flatten()
.filter_map(|thread_dir| {
let file_name = thread_dir.file_name();
let file_name = file_name.to_string_lossy();
let file_name = file_name.trim();
if is_str_numeric(file_name) && file_name != pid_str {
Some(root.join(file_name).to_path_buf())
} else {
None
}
})
.collect::<Vec<_>>();
}
}
Vec::new()
}