[pbs-devel] [PATCH proxmox-backup v2 03/14] tape/drive: clippy fixes

Dominik Csapak d.csapak at proxmox.com
Fri Apr 16 12:28:59 CEST 2021


fixes:
* manual implementation of an assign operation
* using clone on a copy type
* if chain rewritten with match on Ordering
* put part of complex type in type definition
* unnecessary return
* collapsible if chain
* inconsistent underscore

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 src/tape/drive/lto/mod.rs         | 30 +++++++++++++++++-------------
 src/tape/drive/lto/sg_tape.rs     | 18 ++++++++----------
 src/tape/drive/lto/sg_tape/mam.rs | 12 +++++++-----
 src/tape/drive/mod.rs             |  4 +++-
 src/tape/drive/virtual_tape.rs    |  4 ++--
 5 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index 642c3cc7..ac4d9b93 100644
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -225,18 +225,22 @@ impl LtoTapeHandle {
 
         let current_position = self.current_file_number()?;
 
-        if current_position == position {
-            // make sure we are immediated afer the filemark
-            self.sg_tape.space_filemarks(-1)?;
-            self.sg_tape.space_filemarks(1)?;
-        } else if current_position < position {
-            let diff = position - current_position;
-            self.sg_tape.space_filemarks(diff.try_into()?)?;
-        } else {
-            let diff = current_position - position + 1;
-            self.sg_tape.space_filemarks(-diff.try_into()?)?;
-            // move to EOT side of filemark
-            self.sg_tape.space_filemarks(1)?;
+        match current_position.cmp(&position) {
+            std::cmp::Ordering::Equal => {
+                // make sure we are immediated afer the filemark
+                self.sg_tape.space_filemarks(-1)?;
+                self.sg_tape.space_filemarks(1)?;
+            }
+            std::cmp::Ordering::Less => {
+                let diff = position - current_position;
+                self.sg_tape.space_filemarks(diff.try_into()?)?;
+            }
+            std::cmp::Ordering::Greater => {
+                let diff = current_position - position + 1;
+                self.sg_tape.space_filemarks(-diff.try_into()?)?;
+                // move to EOT side of filemark
+                self.sg_tape.space_filemarks(1)?;
+            }
         }
 
         Ok(())
@@ -409,7 +413,7 @@ impl TapeDriver for LtoTapeHandle {
 
                         let mut tape_key = [0u8; 32];
 
-                        let uuid_bytes: [u8; 16] = uuid.as_bytes().clone();
+                        let uuid_bytes: [u8; 16] = *uuid.as_bytes();
 
                         openssl::pkcs5::pbkdf2_hmac(
                             &item.key,
diff --git a/src/tape/drive/lto/sg_tape.rs b/src/tape/drive/lto/sg_tape.rs
index 7faa1639..aba55f4d 100644
--- a/src/tape/drive/lto/sg_tape.rs
+++ b/src/tape/drive/lto/sg_tape.rs
@@ -82,7 +82,7 @@ impl DataCompressionModePage {
         if enable {
             self.flags2 |= 128;
         } else {
-            self.flags2 = self.flags2 & 127;
+            self.flags2 &= 127;
         }
     }
 
@@ -310,10 +310,8 @@ impl SgTape {
         sg_raw.do_command(&cmd)
             .map_err(|err| format_err!("move to EOD failed - {}", err))?;
 
-        if write_missing_eof {
-            if !self.check_filemark()? {
-                self.write_filemarks(1, false)?;
-            }
+        if write_missing_eof && !self.check_filemark()? {
+            self.write_filemarks(1, false)?;
         }
 
         Ok(())
@@ -476,7 +474,7 @@ impl SgTape {
 
     /// Read Volume Statistics
     pub fn volume_statistics(&mut self) -> Result<Lp17VolumeStatistics, Error> {
-        return read_volume_statistics(&mut self.file);
+        read_volume_statistics(&mut self.file)
     }
 
     pub fn set_encryption(
@@ -516,9 +514,9 @@ impl SgTape {
         //println!("WRITE {:?}", data);
 
         match sg_raw.do_out_command(&cmd, data) {
-            Ok(()) => { return Ok(false) }
+            Ok(()) => { Ok(false) }
             Err(ScsiError::Sense(SenseInfo { sense_key: 0, asc: 0, ascq: 2 })) => {
-                return Ok(true); // LEOM
+                Ok(true) // LEOM
             }
             Err(err) => {
                 proxmox::io_bail!("write failed - {}", err);
@@ -608,9 +606,9 @@ impl SgTape {
         }
 
         if let Some(buffer_mode) = buffer_mode {
-            let mut mode = head.flags3 & 0b1_000_1111;
+            let mut mode = head.flags3 & 0b1000_1111;
             if buffer_mode {
-                mode |= 0b0_001_0000;
+                mode |= 0b0001_0000;
             }
             head.flags3 = mode;
         }
diff --git a/src/tape/drive/lto/sg_tape/mam.rs b/src/tape/drive/lto/sg_tape/mam.rs
index 06b81850..37dd1a8e 100644
--- a/src/tape/drive/lto/sg_tape/mam.rs
+++ b/src/tape/drive/lto/sg_tape/mam.rs
@@ -132,11 +132,13 @@ fn decode_mam_attributes(data: &[u8]) -> Result<Vec<MamAttribute>, Error> {
     let expected_len = data_len as usize;
 
 
-    if reader.len() < expected_len {
-        bail!("read_mam_attributes: got unexpected data len ({} != {})", reader.len(), expected_len);
-    } else if reader.len() > expected_len {
-        // Note: Quantum hh7 returns the allocation_length instead of real data_len
-        reader = &data[4..expected_len+4];
+    match reader.len().cmp(&expected_len) {
+        std::cmp::Ordering::Less => bail!("read_mam_attributes: got unexpected data len ({} != {})", reader.len(), expected_len),
+        std::cmp::Ordering::Greater => {
+            // Note: Quantum hh7 returns the allocation_length instead of real data_len
+            reader = &data[4..expected_len+4];
+        }
+        std::cmp::Ordering::Equal => {}, // expected
     }
 
     let mut list = Vec::new();
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index fd8f503d..1b436351 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -231,6 +231,8 @@ pub trait TapeDriver {
     }
 }
 
+type DriveHandleAndName = (Box<dyn MediaChange>, String);
+
 /// Get the media changer (MediaChange + name) associated with a tape drive.
 ///
 /// Returns Ok(None) if the drive has no associated changer device.
@@ -241,7 +243,7 @@ pub trait TapeDriver {
 pub fn media_changer(
     config: &SectionConfigData,
     drive: &str,
-) -> Result<Option<(Box<dyn MediaChange>, String)>, Error> {
+) -> Result<Option<DriveHandleAndName>, Error> {
 
     match config.sections.get(drive) {
         Some((section_type_name, config)) => {
diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index f7ebc0bb..2187e79c 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -215,7 +215,7 @@ impl VirtualTapeHandle {
             Some(VirtualTapeStatus { ref mut pos, .. }) => {
 
                 if count <= *pos {
-                    *pos = *pos - count;
+                    *pos -= count;
                 } else {
                     bail!("backward_space_count_files failed: move before BOT");
                 }
@@ -290,7 +290,7 @@ impl TapeDriver for VirtualTapeHandle {
                 Ok(Box::new(reader))
             }
             None => {
-                return Err(BlockReadError::Error(proxmox::io_format_err!("drive is empty (no tape loaded).")));
+                Err(BlockReadError::Error(proxmox::io_format_err!("drive is empty (no tape loaded).")))
             }
         }
     }
-- 
2.20.1






More information about the pbs-devel mailing list