[pve-devel] [PATCH installer v3 04/15] common: utils: fix clippy warnings

[Christoph Heiss <c.heiss@proxmox.com>]

No functional changes.

Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
---
Changes v2 -> v3:
  * applied new clippy lints (mostly folding if-let-chains)

Changes v1 -> v2:
  * no changes

 proxmox-auto-install-assistant/src/main.rs    | 36 +++++++-------
 .../src/bin/proxmox-auto-installer.rs         |  5 +-
 proxmox-auto-installer/src/utils.rs           | 10 ++--
 .../src/fetch_plugins/http.rs                 |  5 +-
 .../src/fetch_plugins/partition.rs            |  5 +-
 proxmox-installer-common/src/options.rs       | 31 +++++-------
 proxmox-installer-common/src/utils.rs         |  6 +--
 proxmox-post-hook/src/main.rs                 | 11 +++--
 proxmox-tui-installer/src/main.rs             |  5 +-
 proxmox-tui-installer/src/views/bootdisk.rs   |  2 +-
 proxmox-tui-installer/src/views/mod.rs        | 49 ++++++++++---------
 11 files changed, 77 insertions(+), 88 deletions(-)

diff --git a/proxmox-auto-install-assistant/src/main.rs b/proxmox-auto-install-assistant/src/main.rs
index c0d932c..e0f2435 100644
--- a/proxmox-auto-install-assistant/src/main.rs
+++ b/proxmox-auto-install-assistant/src/main.rs
@@ -576,11 +576,11 @@ fn validate_answer(args: &CommandValidateAnswerArgs) -> Result<()> {
             if args.debug {
                 println!("Parsed data from answer file:\n{:#?}", answer);
             }
-            if args.verify_password {
-                if let Err(err) = verify_hashed_password_interactive(&answer) {
-                    eprintln!("{err:#}");
-                    valid = false;
-                }
+            if args.verify_password
+                && let Err(err) = verify_hashed_password_interactive(&answer)
+            {
+                eprintln!("{err:#}");
+                valid = false;
             }
         }
         Err(err) => {
@@ -775,7 +775,7 @@ fn get_iso_uuid(iso: impl AsRef<Path>) -> Result<String> {
         if line.starts_with("-volume_date uuid") {
             uuid = line
                 .split(' ')
-                .last()
+                .next_back()
                 .ok_or_else(|| format_err!("xorriso did behave unexpectedly"))?
                 .replace('\'', "")
                 .trim()
@@ -823,10 +823,10 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
         let mut name = filename;
         let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
         for line in output.lines() {
-            if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX) {
-                if prop != "disk" {
-                    continue 'outer;
-                }
+            if let Some(prop) = line.strip_prefix(PROP_DEVTYP_PREFIX)
+                && prop != "disk"
+            {
+                continue 'outer;
             }
 
             if line.starts_with(PROP_CDROM) || line.starts_with(PROP_ISO9660_FS) {
@@ -837,10 +837,10 @@ fn get_disks() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
                 name = prop.to_owned();
             };
 
-            if let Some(prop) = line.strip_prefix("E: ") {
-                if let Some((key, val)) = prop.split_once('=') {
-                    udev_props.insert(key.to_owned(), val.to_owned());
-                }
+            if let Some(prop) = line.strip_prefix("E: ")
+                && let Some((key, val)) = prop.split_once('=')
+            {
+                udev_props.insert(key.to_owned(), val.to_owned());
             }
         }
 
@@ -867,10 +867,10 @@ fn get_nics() -> Result<BTreeMap<String, BTreeMap<String, String>>> {
         let mut udev_props: BTreeMap<String, String> = BTreeMap::new();
 
         for line in output.lines() {
-            if let Some(prop) = line.strip_prefix("E: ") {
-                if let Some((key, val)) = prop.split_once('=') {
-                    udev_props.insert(key.to_owned(), val.to_owned());
-                }
+            if let Some(prop) = line.strip_prefix("E: ")
+                && let Some((key, val)) = prop.split_once('=')
+            {
+                udev_props.insert(key.to_owned(), val.to_owned());
             }
         }
 
diff --git a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
index a42029f..467ef1b 100644
--- a/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
+++ b/proxmox-auto-installer/src/bin/proxmox-auto-installer.rs
@@ -122,11 +122,10 @@ fn main() -> ExitCode {
         }
     };
 
-    if answer.global.reboot_on_error {
-        if let Err(err) = File::create("/run/proxmox-reboot-on-error") {
+    if answer.global.reboot_on_error
+        && let Err(err) = File::create("/run/proxmox-reboot-on-error") {
             error!("failed to create reboot-on-error flag-file: {err}");
         }
-    }
 
     if answer.global.reboot_mode == RebootMode::PowerOff {
         if let Err(err) = File::create("/run/proxmox-poweroff-after-install") {
diff --git a/proxmox-auto-installer/src/utils.rs b/proxmox-auto-installer/src/utils.rs
index 7d42f2c..2d68829 100644
--- a/proxmox-auto-installer/src/utils.rs
+++ b/proxmox-auto-installer/src/utils.rs
@@ -409,11 +409,10 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
         }
     }
 
-    if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options {
-        if let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) {
+    if let answer::FsOptions::LVM(lvm) = &answer.disks.fs_options
+        && let Some((swapsize, hdsize)) = lvm.swapsize.zip(lvm.hdsize) {
             check_swapsize(swapsize, hdsize)?;
         }
-    }
 
     Ok(())
 }
@@ -421,11 +420,10 @@ pub fn verify_disks_settings(answer: &Answer) -> Result<()> {
 pub fn verify_first_boot_settings(answer: &Answer) -> Result<()> {
     info!("Verifying first boot settings");
 
-    if let Some(first_boot) = &answer.first_boot {
-        if first_boot.source == FirstBootHookSourceMode::FromUrl && first_boot.url.is_none() {
+    if let Some(first_boot) = &answer.first_boot
+        && first_boot.source == FirstBootHookSourceMode::FromUrl && first_boot.url.is_none() {
             bail!("first-boot executable source set to URL, but none specified!");
         }
-    }
 
     Ok(())
 }
diff --git a/proxmox-fetch-answer/src/fetch_plugins/http.rs b/proxmox-fetch-answer/src/fetch_plugins/http.rs
index 9b3d15c..d6331d8 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/http.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/http.rs
@@ -139,11 +139,10 @@ impl FetchFromHTTP {
     fn get_search_domain() -> Result<String> {
         info!("Retrieving default search domain.");
         for line in read_to_string("/etc/resolv.conf")?.lines() {
-            if let Some((key, value)) = line.split_once(' ') {
-                if key == "search" {
+            if let Some((key, value)) = line.split_once(' ')
+                && key == "search" {
                     return Ok(value.trim().into());
                 }
-            }
         }
         bail!("Could not find search domain in resolv.conf.");
     }
diff --git a/proxmox-fetch-answer/src/fetch_plugins/partition.rs b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
index 7a7b17a..f952513 100644
--- a/proxmox-fetch-answer/src/fetch_plugins/partition.rs
+++ b/proxmox-fetch-answer/src/fetch_plugins/partition.rs
@@ -122,11 +122,10 @@ fn mount_proxmoxinst_part(part_label: &str) -> Result<String> {
 fn check_if_mounted(target_path: &str) -> Result<bool> {
     let mounts = fs::read_to_string("/proc/mounts")?;
     for line in mounts.lines() {
-        if let Some(mp) = line.split(' ').nth(1) {
-            if mp == target_path {
+        if let Some(mp) = line.split(' ').nth(1)
+            && mp == target_path {
                 return Ok(true);
             }
-        }
     }
     Ok(false)
 }
diff --git a/proxmox-installer-common/src/options.rs b/proxmox-installer-common/src/options.rs
index 50fd74e..263bcfb 100644
--- a/proxmox-installer-common/src/options.rs
+++ b/proxmox-installer-common/src/options.rs
@@ -104,7 +104,7 @@ impl ZfsRaidLevel {
         match self {
             ZfsRaidLevel::Raid0 => {}
             ZfsRaidLevel::Raid10 => {
-                if disks.len() % 2 != 0 {
+                if !disks.len().is_multiple_of(2) {
                     return Err(format!(
                         "Needs an even number of disks, currently selected: {}",
                         disks.len(),
@@ -515,42 +515,35 @@ impl NetworkOptions {
 
         if let Some(routes) = &network.routes {
             let mut filled = false;
-            if let Some(gw) = &routes.gateway4 {
-                if let Some(iface) = network.interfaces.get(&gw.dev) {
+            if let Some(gw) = &routes.gateway4
+                && let Some(iface) = network.interfaces.get(&gw.dev) {
                     this.ifname.clone_from(&iface.name);
-                    if let Some(addresses) = &iface.addresses {
-                        if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
+                    if let Some(addresses) = &iface.addresses
+                        && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv4()) {
                             this.gateway = gw.gateway;
                             this.address = addr.clone();
                             filled = true;
                         }
-                    }
                 }
-            }
-            if !filled {
-                if let Some(gw) = &routes.gateway6 {
-                    if let Some(iface) = network.interfaces.get(&gw.dev) {
-                        if let Some(addresses) = &iface.addresses {
-                            if let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
+            if !filled
+                && let Some(gw) = &routes.gateway6
+                    && let Some(iface) = network.interfaces.get(&gw.dev)
+                        && let Some(addresses) = &iface.addresses
+                            && let Some(addr) = addresses.iter().find(|addr| addr.is_ipv6()) {
                                 this.ifname.clone_from(&iface.name);
                                 this.gateway = gw.gateway;
                                 this.address = addr.clone();
                             }
-                        }
-                    }
-                }
-            }
         }
 
         // In case no there are no routes defined at all (e.g. no DHCP lease),
         // try to set the interface name to *some* valid values. At least one
         // NIC should always be present here, as the installation will abort
         // earlier in that case, so use the first one enumerated.
-        if this.ifname.is_empty() {
-            if let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
+        if this.ifname.is_empty()
+            && let Some(iface) = network.interfaces.values().min_by_key(|v| v.index) {
                 this.ifname.clone_from(&iface.name);
             }
-        }
 
         this
     }
diff --git a/proxmox-installer-common/src/utils.rs b/proxmox-installer-common/src/utils.rs
index 054a0fd..ffc862e 100644
--- a/proxmox-installer-common/src/utils.rs
+++ b/proxmox-installer-common/src/utils.rs
@@ -130,14 +130,14 @@ fn mask_limit(addr: &IpAddr) -> usize {
 }
 
 fn check_mask_limit(addr: &IpAddr, mask: usize) -> Result<(), CidrAddressParseError> {
-    let limit = mask_limit(&addr);
-    return if mask > limit {
+    let limit = mask_limit(addr);
+    if mask > limit {
         Err(CidrAddressParseError::InvalidMask(
             format!("mask cannot be greater than {limit}").into(),
         ))
     } else {
         Ok(())
-    };
+    }
 }
 
 /// Possible errors that might occur when parsing FQDNs.
diff --git a/proxmox-post-hook/src/main.rs b/proxmox-post-hook/src/main.rs
index bd27121..3897c26 100644
--- a/proxmox-post-hook/src/main.rs
+++ b/proxmox-post-hook/src/main.rs
@@ -547,10 +547,11 @@ impl PostHookInfo {
         for pkg in kernel_pkgs.lines() {
             let parts = pkg.split('|').collect::<Vec<&str>>();
 
-            if let [status, arch, name] = parts[..] {
-                if status.trim() == "ii" && arch.trim() == dpkg_arch {
-                    return Ok(name.trim().to_owned());
-                }
+            if let [status, arch, name] = parts[..]
+                && status.trim() == "ii"
+                && arch.trim() == dpkg_arch
+            {
+                return Ok(name.trim().to_owned());
             }
         }
 
@@ -612,7 +613,7 @@ impl PostHookInfo {
 /// # Arguments
 ///
 /// * `callback` - Callback to call with the absolute path where the chroot environment root is
-///                mounted.
+///   mounted.
 fn with_chroot<R, F: FnOnce(&str) -> Result<R>>(callback: F) -> Result<R> {
     let ec = Command::new("proxmox-chroot")
         .arg("prepare")
diff --git a/proxmox-tui-installer/src/main.rs b/proxmox-tui-installer/src/main.rs
index 15ee5d3..3379e7c 100644
--- a/proxmox-tui-installer/src/main.rs
+++ b/proxmox-tui-installer/src/main.rs
@@ -188,11 +188,10 @@ fn installer_setup_late(siv: &mut Cursive) {
 
     if !state.in_test_mode {
         let kmap_id = &state.options.timezone.kb_layout;
-        if let Some(kmap) = state.locales.kmap.get(kmap_id) {
-            if let Err(err) = system::set_keyboard_layout(kmap) {
+        if let Some(kmap) = state.locales.kmap.get(kmap_id)
+            && let Err(err) = system::set_keyboard_layout(kmap) {
                 display_setup_warning(siv, &format!("Failed to apply keyboard layout: {err}"));
             }
-        }
     }
 
     if state.runtime_info.total_memory < 1024 {
diff --git a/proxmox-tui-installer/src/views/bootdisk.rs b/proxmox-tui-installer/src/views/bootdisk.rs
index 9f6d235..7e3680b 100644
--- a/proxmox-tui-installer/src/views/bootdisk.rs
+++ b/proxmox-tui-installer/src/views/bootdisk.rs
@@ -183,7 +183,7 @@ impl AdvancedBootdiskOptionsView {
     /// # Arguments
     /// * `siv` - Cursive instance
     /// * `fstype` - The chosen filesystem type by the user, for which the UI should be
-    ///              updated accordingly
+    ///   updated accordingly
     /// * `options_ref` - [`BootdiskOptionsRef`] where advanced disk options should be saved to
     fn fstype_on_submit(siv: &mut Cursive, fstype: &FsType, options_ref: BootdiskOptionsRef) {
         let state = siv.user_data::<InstallerState>().unwrap();
diff --git a/proxmox-tui-installer/src/views/mod.rs b/proxmox-tui-installer/src/views/mod.rs
index 537e3ed..3417191 100644
--- a/proxmox-tui-installer/src/views/mod.rs
+++ b/proxmox-tui-installer/src/views/mod.rs
@@ -109,10 +109,10 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
     pub fn get_content(&self) -> Result<T, <T as FromStr>::Err> {
         let content = self.inner().get_content();
 
-        if content.is_empty() {
-            if let Some(placeholder) = self.placeholder {
-                return Ok(placeholder);
-            }
+        if content.is_empty()
+            && let Some(placeholder) = self.placeholder
+        {
+            return Ok(placeholder);
         }
 
         assert!(!(self.allow_empty && self.placeholder.is_none()));
@@ -137,17 +137,17 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
 
     fn check_bounds(&mut self, original: Arc<String>, result: EventResult) -> EventResult {
         // Check if the new value is actually valid according to the max value, if set
-        if let Some(max) = self.max_value {
-            if let Ok(val) = self.get_content() {
-                if result.is_consumed() && val > max {
-                    // Restore the original value, before the insert
-                    let cb = self.inner_mut().set_content((*original).clone());
-                    return EventResult::with_cb_once(move |siv| {
-                        result.process(siv);
-                        cb(siv);
-                    });
-                }
-            }
+        if let Some(max) = self.max_value
+            && let Ok(val) = self.get_content()
+            && result.is_consumed()
+            && val > max
+        {
+            // Restore the original value, before the insert
+            let cb = self.inner_mut().set_content((*original).clone());
+            return EventResult::with_cb_once(move |siv| {
+                result.process(siv);
+                cb(siv);
+            });
         }
 
         result
@@ -182,7 +182,7 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
     ///
     /// # Arguments
     /// * `content` - New, stringified content for the inner [`EditView`]. Must be a valid value
-    ///               according to the container type `T`.
+    ///   according to the container type `T`.
     fn content_inner(mut self, content: &str) -> Self {
         let mut inner = EditView::new();
         std::mem::swap(self.inner_mut(), &mut inner);
@@ -198,15 +198,16 @@ impl<T: Copy + ToString + FromStr + PartialOrd> NumericEditView<T> {
     fn wrap_draw_inner(&self, printer: &Printer) {
         self.view.draw(printer);
 
-        if self.inner().get_content().is_empty() && !printer.focused {
-            if let Some(placeholder) = self.placeholder {
-                let placeholder = placeholder.to_string();
+        if self.inner().get_content().is_empty()
+            && !printer.focused
+            && let Some(placeholder) = self.placeholder
+        {
+            let placeholder = placeholder.to_string();
 
-                printer.with_color(
-                    (BaseColor::Blue.light(), BaseColor::Blue.dark()).into(),
-                    |printer| printer.print((0, 0), &placeholder),
-                );
-            }
+            printer.with_color(
+                (BaseColor::Blue.light(), BaseColor::Blue.dark()).into(),
+                |printer| printer.print((0, 0), &placeholder),
+            );
         }
     }
 }
-- 
2.51.0



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel