From a5b95ae8b26c720892cfe2dfbe3b52a6f6eaf546 Mon Sep 17 00:00:00 2001 From: Clement Tsang <34804052+ClementTsang@users.noreply.github.com> Date: Thu, 1 Oct 2020 01:46:09 -0400 Subject: [PATCH] bug: use cmdline for Linux proc name if >=15 chars (#261) This was the cause of some process names getting cut off and looking weird for Linux (and Linux only, I'm not directly responsible for the other OSes). This also adds spaces in between command line flags. Before, they were usually separated by either spaces (which looked fine) or null terminators (which meant it looked like something was broken). --- CHANGELOG.md | 2 ++ src/app/data_harvester.rs | 2 ++ src/app/data_harvester/processes.rs | 41 ++++++++++++++++++++++++----- src/bin/main.rs | 6 +++++ src/lib.rs | 9 ++++++- src/options.rs | 6 ++--- 6 files changed, 56 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e38e9b4..5f3ad2f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [#253](https://github.com/ClementTsang/bottom/pull/253): Expanding a widget no longer overrides the widget title colour. +- [#261](https://github.com/ClementTsang/bottom/pull/261): Fixed process names occasionally showing up as truncated, due to only using `/proc//stat` as our data source. + ## [0.4.7] - 2020-08-26 ### Bug Fixes diff --git a/src/app/data_harvester.rs b/src/app/data_harvester.rs index bd0c98e0..285c4823 100644 --- a/src/app/data_harvester.rs +++ b/src/app/data_harvester.rs @@ -122,8 +122,10 @@ impl Default for DataCollector { impl DataCollector { pub fn init(&mut self) { self.mem_total_kb = self.sys.get_total_memory(); + trace!("Total memory in KB: {}", self.mem_total_kb); if self.widgets_to_harvest.use_battery { + trace!("First run battery vec creation."); if let Ok(battery_manager) = Manager::new() { if let Ok(batteries) = battery_manager.batteries() { let battery_list: Vec = batteries.filter_map(Result::ok).collect(); diff --git a/src/app/data_harvester/processes.rs b/src/app/data_harvester/processes.rs index 64988338..62789146 100644 --- a/src/app/data_harvester/processes.rs +++ b/src/app/data_harvester/processes.rs @@ -11,6 +11,9 @@ use std::collections::{hash_map::RandomState, HashMap}; #[cfg(not(target_os = "linux"))] use sysinfo::{ProcessExt, ProcessorExt, System, SystemExt}; +/// Maximum character length of a /proc//stat process name. +const MAX_STAT_NAME_LEN: usize = 15; + // TODO: Add value so we know if it's sorted ascending or descending by default? #[derive(Clone, PartialEq, Eq, Hash, Debug)] pub enum ProcessSorting { @@ -251,7 +254,9 @@ fn read_proc( .entry(pid) .or_insert_with(|| PrevProcDetails::new(pid)); let stat_results = read_path_contents(&pid_stat.proc_stat_path)?; - let name = stat_results + + // truncated_name may potentially be cut! Hence why we do the bit of code after... + let truncated_name = stat_results .splitn(2, '(') .collect::>() .last() @@ -261,12 +266,36 @@ fn read_proc( .last() .ok_or(BottomError::MinorError)? .to_string(); - let command = { - let cmd = read_path_contents(&pid_stat.proc_cmdline_path)?; // FIXME: [PROC] Use full proc name all the time - if cmd.trim().is_empty() { - format!("[{}]", name) + let (command, name) = { + let cmd = read_path_contents(&pid_stat.proc_cmdline_path)?; + let trimmed_cmd = cmd.trim(); + if trimmed_cmd.is_empty() { + (format!("[{}]", truncated_name), truncated_name) } else { - cmd + // We split by spaces and null terminators. + let separated_strings = trimmed_cmd + .split_terminator(|c| c == '\0' || c == ' ') + .collect::>(); + + ( + separated_strings.join(" "), + if truncated_name.len() >= MAX_STAT_NAME_LEN { + if let Some(first_part) = separated_strings.first() { + // We're only interested in the executable part... not the file path. + // That's for command. + first_part + .split('/') + .collect::>() + .last() + .unwrap_or(&truncated_name.as_str()) + .to_string() + } else { + truncated_name + } + } else { + truncated_name + }, + ) } }; let stat = stat_results diff --git a/src/bin/main.rs b/src/bin/main.rs index 81ac2041..50881529 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -42,8 +42,10 @@ fn main() -> Result<()> { let config_path = read_config(matches.value_of("config_location")) .context("Unable to access the given config file location.")?; + trace!("Config path: {:?}", config_path); let mut config: Config = create_or_get_config(&config_path) .context("Unable to properly parse or create the config file.")?; + trace!("Current config: {:#?}", config); // Get widget layout separately let (widget_layout, default_widget_id, default_widget_type_option) = @@ -75,13 +77,17 @@ fn main() -> Result<()> { // Cleaning loop { let cleaning_sender = sender.clone(); + trace!("Initializing cleaning thread..."); thread::spawn(move || loop { thread::sleep(Duration::from_millis( constants::STALE_MAX_MILLISECONDS + 5000, )); + trace!("Sending cleaning signal..."); if cleaning_sender.send(BottomEvent::Clean).is_err() { + trace!("Failed to send cleaning signal. Halting cleaning thread loop."); break; } + trace!("Cleaning signal sent without errors."); }); } diff --git a/src/lib.rs b/src/lib.rs index 7388fe83..605b79b7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -615,17 +615,23 @@ pub fn create_collection_thread( thread::spawn(move || { trace!("Spawned collection thread."); let mut data_state = data_harvester::DataCollector::default(); + trace!("Created initial data state."); data_state.set_collected_data(used_widget_set); + trace!("Set collected data."); data_state.set_temperature_type(temp_type); + trace!("Set initial temp type."); data_state.set_use_current_cpu_total(use_current_cpu_total); + trace!("Set current CPU total."); data_state.set_show_average_cpu(show_average_cpu); + trace!("Set showing average CPU."); data_state.init(); + trace!("Data state is now fully initialized."); loop { trace!("Collecting..."); let mut update_time = update_rate_in_milliseconds; if let Ok(message) = reset_receiver.try_recv() { - trace!("Received message: {:?}", message); + trace!("Received message in collection thread: {:?}", message); match message { CollectionThreadEvent::Reset => { data_state.data.cleanup(); @@ -650,6 +656,7 @@ pub fn create_collection_thread( trace!("Collection thread done updating. Sending data now..."); data_state.data = data_harvester::Data::default(); if sender.send(event).is_err() { + trace!("Error sending from collection thread..."); break; } trace!("No problem sending from collection thread!"); diff --git a/src/options.rs b/src/options.rs index 0f85a611..27d722ae 100644 --- a/src/options.rs +++ b/src/options.rs @@ -20,7 +20,7 @@ pub mod layout_options; use anyhow::{Context, Result}; -#[derive(Clone, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct Config { pub flags: Option, pub colors: Option, @@ -154,7 +154,7 @@ impl WidgetIdEnabled { } } -#[derive(Clone, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct ConfigColours { pub table_header_color: Option, pub all_cpu_color: Option, @@ -176,7 +176,7 @@ pub struct ConfigColours { pub battery_colors: Option>, } -#[derive(Clone, Default, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct IgnoreList { pub is_list_ignored: bool, pub list: Vec,