battery: deduplicate frequency validation logic

src/battery.rs

@@ -164,14 +164,16 @@ fn write_sysfs_value(path: impl AsRef<Path>, value: &str) -> Result<()> {
 /// Read a value from a sysfs file
 fn read_sysfs_value(path: impl AsRef<Path>) -> Result<String> {
     let p = path.as_ref();
-    fs::read_to_string(p).map_err(|e| {
+    fs::read_to_string(p)
-        let error_msg = format!("Path: {:?}, Error: {}", p.display(), e);
+        .map_err(|e| {
-        if e.kind() == io::ErrorKind::PermissionDenied {
+            let error_msg = format!("Path: {:?}, Error: {}", p.display(), e);
-            ControlError::PermissionDenied(error_msg)
+            if e.kind() == io::ErrorKind::PermissionDenied {
-        } else {
+                ControlError::PermissionDenied(error_msg)
-            ControlError::ReadError(error_msg)
+            } else {
-        }
+                ControlError::ReadError(error_msg)
-    }).map(|s| s.trim().to_string())
+            }
+        })
+        .map(|s| s.trim().to_string())
 }
 
 /// Safely check if a path exists and is writable
@@ -225,7 +227,7 @@ fn apply_thresholds_to_batteries(
         let stop_result = write_sysfs_value(&stop_path, &stop_threshold.to_string());
 
         // Only proceed to set start threshold if stop threshold was set successfully
-        if let Ok(()) = stop_result {
+        if matches!(stop_result, Ok(())) {
             let start_result = write_sysfs_value(&start_path, &start_threshold.to_string());
 
             match start_result {
@@ -246,7 +248,10 @@ fn apply_thresholds_to_batteries(
                                 battery.name, re
                             );
                         } else {
-                            debug!("Restored previous stop threshold ({}) for battery '{}'", prev_stop, battery.name);
+                            debug!(
+                                "Restored previous stop threshold ({}) for battery '{}'",
+                                prev_stop, battery.name
+                            );
                         }
                     }
 

src/main.rs

@@ -358,17 +358,9 @@ fn main() {
         }
         Some(Commands::SetMinFreq { freq_mhz, core_id }) => {
             // Basic validation for reasonable CPU frequency values
-            if freq_mhz == 0 {
+            if let Err(e) = validate_freq(freq_mhz, "Minimum") {
-                error!("Minimum frequency cannot be zero");
+                error!("{e}");
-                Err(Box::new(ControlError::InvalidValueError(
+                Err(e)
-                    "Minimum frequency cannot be zero".to_string(),
-                )) as Box<dyn std::error::Error>)
-            } else if freq_mhz > 10000 {
-                // Extremely high value unlikely to be valid
-                error!("Minimum frequency ({freq_mhz} MHz) is unreasonably high");
-                Err(Box::new(ControlError::InvalidValueError(format!(
-                    "Minimum frequency ({freq_mhz} MHz) is unreasonably high"
-                ))) as Box<dyn std::error::Error>)
             } else {
                 cpu::set_min_frequency(freq_mhz, core_id)
                     .map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
@@ -376,17 +368,9 @@ fn main() {
         }
         Some(Commands::SetMaxFreq { freq_mhz, core_id }) => {
             // Basic validation for reasonable CPU frequency values
-            if freq_mhz == 0 {
+            if let Err(e) = validate_freq(freq_mhz, "Maximum") {
-                error!("Maximum frequency cannot be zero");
+                error!("{e}");
-                Err(Box::new(ControlError::InvalidValueError(
+                Err(e)
-                    "Maximum frequency cannot be zero".to_string(),
-                )) as Box<dyn std::error::Error>)
-            } else if freq_mhz > 10000 {
-                // Extremely high value unlikely to be valid
-                error!("Maximum frequency ({freq_mhz} MHz) is unreasonably high");
-                Err(Box::new(ControlError::InvalidValueError(format!(
-                    "Maximum frequency ({freq_mhz} MHz) is unreasonably high"
-                ))) as Box<dyn std::error::Error>)
             } else {
                 cpu::set_max_frequency(freq_mhz, core_id)
                     .map_err(|e| Box::new(e) as Box<dyn std::error::Error>)
@@ -497,3 +481,21 @@ fn init_logger() {
         debug!("Logger initialized with RUST_LOG={env_log}");
     });
 }
+
+/// Validate CPU frequency input values
+fn validate_freq(freq_mhz: u32, label: &str) -> Result<(), Box<dyn std::error::Error>> {
+    if freq_mhz == 0 {
+        error!("{label} frequency cannot be zero");
+        Err(Box::new(ControlError::InvalidValueError(format!(
+            "{label} frequency cannot be zero"
+        ))) as Box<dyn std::error::Error>)
+    } else if freq_mhz > 10000 {
+        // Extremely high value unlikely to be valid
+        error!("{label} frequency ({freq_mhz} MHz) is unreasonably high");
+        Err(Box::new(ControlError::InvalidValueError(format!(
+            "{label} frequency ({freq_mhz} MHz) is unreasonably high"
+        ))) as Box<dyn std::error::Error>)
+    } else {
+        Ok(())
+    }
+}