Skip to content

Commit

Permalink
bug: Fix process CPU calculation if /proc/stat CPU line has less valu…
Browse files Browse the repository at this point in the history
…es than expected (#637)

Addresses a potential case where processing would fail if there were missing values from the CPU line of `/proc/stat`, and allows it to successfully return.
  • Loading branch information
ClementTsang authored Dec 21, 2021
1 parent d6a112b commit d32a74e
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 24 deletions.
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.6.6]/[0.7.0] - Unreleased
## [0.6.6] - Unreleased

## Bug Fixes

- [#637](https://github.com/ClementTsang/bottom/pull/637): Fix process CPU calculation if /proc/stat CPU line has less values than expected

## [0.6.5] - 2021-12-19

Expand Down
92 changes: 69 additions & 23 deletions src/app/data_harvester/processes/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,40 +36,42 @@ impl PrevProcDetails {
}
}

fn calculate_idle_values(line: String) -> (f64, f64) {
/// Converts a `Option<&str>` value to an f64. If it fails to parse or is `None`, then it will return `0_f64`.
fn str_to_f64(val: Option<&str>) -> f64 {
val.and_then(|v| v.parse::<f64>().ok()).unwrap_or(0_f64)
}

let mut val = line.split_whitespace();
let user = str_to_f64(val.next());
let nice: f64 = str_to_f64(val.next());
let system: f64 = str_to_f64(val.next());
let idle: f64 = str_to_f64(val.next());
let iowait: f64 = str_to_f64(val.next());
let irq: f64 = str_to_f64(val.next());
let softirq: f64 = str_to_f64(val.next());
let steal: f64 = str_to_f64(val.next());

// Note we do not get guest/guest_nice, as they are calculated as part of user/nice respectively

let idle = idle + iowait;
let non_idle = user + nice + system + irq + softirq + steal;

(idle, non_idle)
}

fn cpu_usage_calculation(
prev_idle: &mut f64, prev_non_idle: &mut f64,
) -> error::Result<(f64, f64)> {
use std::io::prelude::*;
use std::io::BufReader;

// From SO answer: https://stackoverflow.com/a/23376195

let mut reader = BufReader::new(std::fs::File::open("/proc/stat")?);
let mut first_line = String::new();
reader.read_line(&mut first_line)?;

let val = first_line.split_whitespace().collect::<Vec<&str>>();

// SC in case that the parsing will fail due to length:
if val.len() <= 10 {
return Err(error::BottomError::InvalidIo(format!(
"CPU parsing will fail due to too short of a return value; saw {} values, expected 10 values.",
val.len()
)));
}

let user: f64 = val[1].parse::<_>().unwrap_or(0_f64);
let nice: f64 = val[2].parse::<_>().unwrap_or(0_f64);
let system: f64 = val[3].parse::<_>().unwrap_or(0_f64);
let idle: f64 = val[4].parse::<_>().unwrap_or(0_f64);
let iowait: f64 = val[5].parse::<_>().unwrap_or(0_f64);
let irq: f64 = val[6].parse::<_>().unwrap_or(0_f64);
let softirq: f64 = val[7].parse::<_>().unwrap_or(0_f64);
let steal: f64 = val[8].parse::<_>().unwrap_or(0_f64);
let guest: f64 = val[9].parse::<_>().unwrap_or(0_f64);

let idle = idle + iowait;
let non_idle = user + nice + system + irq + softirq + steal + guest;
let (idle, non_idle) = calculate_idle_values(first_line);

let total = idle + non_idle;
let prev_total = *prev_idle + *prev_non_idle;
Expand Down Expand Up @@ -294,3 +296,47 @@ pub fn get_process_data(
))
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_proc_cpu_parse() {
assert_eq!(
(100_f64, 200_f64),
calculate_idle_values("100 0 100 100".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 4 values"
);
assert_eq!(
(120_f64, 200_f64),
calculate_idle_values("100 0 100 100 20".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 5 values"
);
assert_eq!(
(120_f64, 230_f64),
calculate_idle_values("100 0 100 100 20 30".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 6 values"
);
assert_eq!(
(120_f64, 270_f64),
calculate_idle_values("100 0 100 100 20 30 40".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 7 values"
);
assert_eq!(
(120_f64, 320_f64),
calculate_idle_values("100 0 100 100 20 30 40 50".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 8 values"
);
assert_eq!(
(120_f64, 320_f64),
calculate_idle_values("100 0 100 100 20 30 40 50 100".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 9 values"
);
assert_eq!(
(120_f64, 320_f64),
calculate_idle_values("100 0 100 100 20 30 40 50 100 200".to_string()),
"Failed to properly calculate idle/non-idle for /proc/stat CPU with 10 values"
);
}
}

1 comment on commit d32a74e

@vercel
Copy link

@vercel vercel bot commented on d32a74e Dec 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.