From 78a05bc68377497dfd52eaee37c7a5e3bedd71b1 Mon Sep 17 00:00:00 2001 From: ClementTsang Date: Sun, 8 Mar 2020 22:19:57 -0400 Subject: [PATCH] Fixes bug with too large inputs causing a panic We would prefer a more graceful error message stating what went wrong. Caught by the Travis test. --- README.md | 2 +- src/main.rs | 6 ++- src/options.rs | 36 ++++++++-------- tests/{arg_rate_tests.rs => arg_tests.rs} | 52 +++++++++++++++++++++++ 4 files changed, 75 insertions(+), 21 deletions(-) rename tests/{arg_rate_tests.rs => arg_tests.rs} (66%) diff --git a/README.md b/README.md index 4dda210b..35dd41b4 100644 --- a/README.md +++ b/README.md @@ -142,7 +142,7 @@ Run using `btm`. - `-t`, `--default_time_value` will set the default time interval graphs will display to (in milliseconds). Lowest is 30 seconds, defaults to 60 seconds. -- `-i`, `--time_delta` will set the amount each zoom in/out action will change the time interval of a graph (in milliseconds). Lowest is 1 second, defaults to 15 seconds. +- `-d`, `--time_delta` will set the amount each zoom in/out action will change the time interval of a graph (in milliseconds). Lowest is 1 second, defaults to 15 seconds. ### Keybindings diff --git a/src/main.rs b/src/main.rs index 9d09c262..56c1d21b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -87,8 +87,10 @@ fn get_matches() -> clap::ArgMatches<'static> { (@arg REGEX_DEFAULT: -R --regex "Use regex in searching by default.") (@arg SHOW_DISABLED_DATA: -s --show_disabled_data "Show disabled data entries.") (@arg DEFAULT_TIME_VALUE: -t --default_time_value +takes_value "Default time value for graphs in milliseconds; minimum is 30s, defaults to 60s.") - (@arg TIME_DELTA: -i --time_delta +takes_value "The amount changed upon zooming in/out in milliseconds; minimum is 1s, defaults to 15s.") - (@group DEFAULT_WIDGET => + (@arg TIME_DELTA: -d --time_delta +takes_value "The amount changed upon zooming in/out in milliseconds; minimum is 1s, defaults to 15s.") + (@arg HIDE_TIME: --hide_time "Completely hide the time scaling") + (@arg AUTOHIDE_TIME: --autohide_time "Automatically hide the time scaling in graphs after being shown for a brief moment when zoomed in/out. If time is disabled then this will have no effect.") + (@group DEFAULT_WIDGET => (@arg CPU_WIDGET: --cpu_default "Selects the CPU widget to be selected by default.") (@arg MEM_WIDGET: --memory_default "Selects the memory widget to be selected by default.") (@arg DISK_WIDGET: --disk_default "Selects the disk widget to be selected by default.") diff --git a/src/options.rs b/src/options.rs index 77884e78..449cb66e 100644 --- a/src/options.rs +++ b/src/options.rs @@ -56,15 +56,15 @@ pub fn get_update_rate_in_milliseconds( update_rate: &Option<&str>, config: &Config, ) -> error::Result { let update_rate_in_milliseconds = if let Some(update_rate) = update_rate { - update_rate.parse::()? + update_rate.parse::()? } else if let Some(flags) = &config.flags { if let Some(rate) = flags.rate { - rate + rate as u128 } else { - DEFAULT_REFRESH_RATE_IN_MILLISECONDS + DEFAULT_REFRESH_RATE_IN_MILLISECONDS as u128 } } else { - DEFAULT_REFRESH_RATE_IN_MILLISECONDS + DEFAULT_REFRESH_RATE_IN_MILLISECONDS as u128 }; if update_rate_in_milliseconds < 250 { @@ -77,7 +77,7 @@ pub fn get_update_rate_in_milliseconds( )); } - Ok(update_rate_in_milliseconds) + Ok(update_rate_in_milliseconds as u64) } pub fn get_temperature_option( @@ -184,15 +184,15 @@ pub fn get_default_time_value_option( matches: &clap::ArgMatches<'static>, config: &Config, ) -> error::Result { let default_time = if let Some(default_time_value) = matches.value_of("DEFAULT_TIME_VALUE") { - default_time_value.parse::()? + default_time_value.parse::()? } else if let Some(flags) = &config.flags { if let Some(default_time_value) = flags.default_time_value { - default_time_value + default_time_value as u128 } else { - DEFAULT_TIME_MILLISECONDS + DEFAULT_TIME_MILLISECONDS as u128 } } else { - DEFAULT_TIME_MILLISECONDS + DEFAULT_TIME_MILLISECONDS as u128 }; if default_time < 30000 { @@ -205,35 +205,35 @@ pub fn get_default_time_value_option( )); } - Ok(default_time) + Ok(default_time as u64) } pub fn get_time_interval_option( matches: &clap::ArgMatches<'static>, config: &Config, ) -> error::Result { let time_interval = if let Some(time_interval) = matches.value_of("TIME_DELTA") { - time_interval.parse::()? + time_interval.parse::()? } else if let Some(flags) = &config.flags { if let Some(time_interval) = flags.time_delta { - time_interval + time_interval as u128 } else { - TIME_CHANGE_MILLISECONDS + TIME_CHANGE_MILLISECONDS as u128 } } else { - TIME_CHANGE_MILLISECONDS + TIME_CHANGE_MILLISECONDS as u128 }; if time_interval < 1000 { return Err(BottomError::InvalidArg( - "Please set your time interval to be at least 1 second.".to_string(), + "Please set your time delta to be at least 1 second.".to_string(), )); - } else if time_interval as u128 > std::u64::MAX as u128 { + } else if time_interval > std::u64::MAX as u128 { return Err(BottomError::InvalidArg( - "Please set your time interval to be at most unsigned INT_MAX.".to_string(), + "Please set your time delta to be at most unsigned INT_MAX.".to_string(), )); } - Ok(time_interval) + Ok(time_interval as u64) } pub fn enable_app_grouping(matches: &clap::ArgMatches<'static>, config: &Config, app: &mut App) { diff --git a/tests/arg_rate_tests.rs b/tests/arg_tests.rs similarity index 66% rename from tests/arg_rate_tests.rs rename to tests/arg_tests.rs index 7f22431c..f184b811 100644 --- a/tests/arg_rate_tests.rs +++ b/tests/arg_tests.rs @@ -34,6 +34,58 @@ fn test_small_rate() -> Result<(), Box> { Ok(()) } +#[test] +fn test_large_default_time() -> Result<(), Box> { + Command::new(get_os_binary_loc()) + .arg("-t") + .arg("18446744073709551616") + .assert() + .failure() + .stderr(predicate::str::contains( + "Please set your default value to be at most unsigned INT_MAX.", + )); + Ok(()) +} + +#[test] +fn test_small_default_time() -> Result<(), Box> { + Command::new(get_os_binary_loc()) + .arg("-t") + .arg("900") + .assert() + .failure() + .stderr(predicate::str::contains( + "Please set your default value to be at least 30 seconds.", + )); + Ok(()) +} + +#[test] +fn test_large_delta_time() -> Result<(), Box> { + Command::new(get_os_binary_loc()) + .arg("-d") + .arg("18446744073709551616") + .assert() + .failure() + .stderr(predicate::str::contains( + "Please set your time delta to be at most unsigned INT_MAX.", + )); + Ok(()) +} + +#[test] +fn test_small_delta_time() -> Result<(), Box> { + Command::new(get_os_binary_loc()) + .arg("-d") + .arg("900") + .assert() + .failure() + .stderr(predicate::str::contains( + "Please set your time delta to be at least 1 second.", + )); + Ok(()) +} + #[test] fn test_large_rate() -> Result<(), Box> { Command::new(get_os_binary_loc())