[pbs-devel] [PATCH proxmox-backup 00/12] clippy fixes

[pbs-devel] [PATCH proxmox-backup 00/12] clippy fixes

[Dominik Csapak]

various clippy fixes all over the place
if the separation is not ok, i can send them ofc separated
differently

(they were not too many of the same type to group by warning,
and too spread out to group by file, so i settled for some middle
ground)

Dominik Csapak (12):
  api2/tape: clippy fixes
  tape/changer: clippy fixes
  tape/drive: clippy fixes
  tape/media_*: clippy fixes
  tape/pool_writer: clippy fixes
  backup: clippy fixes
  pxar: clippy fix `or_fun_call`
  bin: clippy fixes
  tape/*: clippy fixes
  tools: clippy fixes
  config/tape_encryption_keys: clippy fixes
  server/worker_task: clippy fix

 src/api2/config/tape_backup_job.rs           |  1 +
 src/api2/tape/backup.rs                      |  2 +-
 src/api2/tape/media.rs                       | 42 ++++++++++----------
 src/api2/tape/restore.rs                     | 12 +++---
 src/backup/key_derivation.rs                 |  6 +--
 src/backup/manifest.rs                       |  1 +
 src/bin/docgen.rs                            |  2 +-
 src/bin/pmt.rs                               |  2 +-
 src/bin/pmtx.rs                              |  2 +-
 src/config/tape_encryption_keys.rs           |  8 ++--
 src/pxar/extract.rs                          |  4 +-
 src/server/worker_task.rs                    |  2 +-
 src/tape/changer/online_status_map.rs        |  4 +-
 src/tape/changer/sg_pt_changer.rs            |  2 +-
 src/tape/drive/linux_tape.rs                 |  2 +-
 src/tape/drive/mam.rs                        | 12 +++---
 src/tape/drive/mod.rs                        |  4 +-
 src/tape/drive/virtual_tape.rs               |  2 +-
 src/tape/file_formats/multi_volume_writer.rs |  2 +-
 src/tape/inventory.rs                        |  2 +-
 src/tape/media_catalog.rs                    | 10 +++--
 src/tape/media_pool.rs                       | 16 +++-----
 src/tape/media_set.rs                        |  4 ++
 src/tape/pool_writer/catalog_set.rs          |  1 +
 src/tape/pool_writer/mod.rs                  |  4 +-
 src/tape/pool_writer/new_chunks_iterator.rs  |  4 +-
 src/tools/fs.rs                              |  3 ++
 src/tools/sgutils2.rs                        | 18 ++++-----
 28 files changed, 93 insertions(+), 81 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 01/12] api2/tape: clippy fixes

[Dominik Csapak]

fixes:
* too_many_arguments
* `if let Err(_) = ` => `.is_err()`
* combine if branches
* remove unnecessary lifetime
* remove unnecessary return
* `.len() == 0` => `.is_empty()`
* `find().is_some()` => `.any()`

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/config/tape_backup_job.rs |  1 +
 src/api2/tape/backup.rs            |  2 +-
 src/api2/tape/media.rs             | 42 ++++++++++++++----------------
 src/api2/tape/restore.rs           | 12 ++++-----
 4 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/src/api2/config/tape_backup_job.rs b/src/api2/config/tape_backup_job.rs
index caea4e18..70d1f6af 100644
--- a/src/api2/config/tape_backup_job.rs
+++ b/src/api2/config/tape_backup_job.rs
@@ -219,6 +219,7 @@ pub enum DeletableProperty {
     },
 )]
 /// Update the tape backup job
+#[allow(clippy::too_many_arguments)]
 pub fn update_tape_backup_job(
     id: String,
     store: Option<String>,
diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs
index ec35038a..25d746fb 100644
--- a/src/api2/tape/backup.rs
+++ b/src/api2/tape/backup.rs
@@ -601,7 +601,7 @@ pub fn backup_snapshot(
         }
     }
 
-    if let Err(_) = reader_thread.join() {
+    if reader_thread.join().is_err() {
         bail!("chunk reader thread failed");
     }
 
diff --git a/src/api2/tape/media.rs b/src/api2/tape/media.rs
index 811fcb7e..b1497fb3 100644
--- a/src/api2/tape/media.rs
+++ b/src/api2/tape/media.rs
@@ -173,32 +173,30 @@ pub async fn list_media(
     let inventory = Inventory::load(status_path)?;
 
     let privs = user_info.lookup_privs(&auth_id, &["tape", "pool"]);
-    if (privs & PRIV_TAPE_AUDIT) != 0  {
-        if pool.is_none() {
+    if (privs & PRIV_TAPE_AUDIT) != 0 && pool.is_none() {
 
-            for media_id in inventory.list_unassigned_media() {
+        for media_id in inventory.list_unassigned_media() {
 
-                let (mut status, location) = inventory.status_and_location(&media_id.label.uuid);
+            let (mut status, location) = inventory.status_and_location(&media_id.label.uuid);
 
-                if status == MediaStatus::Unknown {
-                    status = MediaStatus::Writable;
-                }
-
-                list.push(MediaListEntry {
-                    uuid: media_id.label.uuid.clone(),
-                    ctime: media_id.label.ctime,
-                    label_text: media_id.label.label_text.to_string(),
-                    location,
-                    status,
-                    catalog: true, // empty, so we do not need a catalog
-                    expired: false,
-                    media_set_uuid: None,
-                    media_set_name: None,
-                    media_set_ctime: None,
-                    seq_nr: None,
-                    pool: None,
-                });
+            if status == MediaStatus::Unknown {
+                status = MediaStatus::Writable;
             }
+
+            list.push(MediaListEntry {
+                uuid: media_id.label.uuid.clone(),
+                ctime: media_id.label.ctime,
+                label_text: media_id.label.label_text.to_string(),
+                location,
+                status,
+                catalog: true, // empty, so we do not need a catalog
+                expired: false,
+                media_set_uuid: None,
+                media_set_name: None,
+                media_set_ctime: None,
+                seq_nr: None,
+                pool: None,
+            });
         }
     }
 
diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 78aac73c..d8b81496 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -136,7 +136,7 @@ impl TryFrom<String> for DataStoreMap {
 }
 
 impl DataStoreMap {
-    fn used_datastores<'a>(&self) -> HashSet<&str> {
+    fn used_datastores(&self) -> HashSet<&str> {
         let mut set = HashSet::new();
         for store in self.map.values() {
             set.insert(store.name());
@@ -157,7 +157,7 @@ impl DataStoreMap {
             return Some(&store);
         }
 
-        return None;
+        None
     }
 }
 
@@ -211,7 +211,7 @@ pub fn restore(
     let store_map = DataStoreMap::try_from(store)
         .map_err(|err| format_err!("cannot parse store mapping: {}", err))?;
     let used_datastores = store_map.used_datastores();
-    if used_datastores.len() == 0 {
+    if used_datastores.is_empty() {
         bail!("no datastores given");
     }
 
@@ -351,6 +351,7 @@ pub fn restore(
 }
 
 /// Request and restore complete media without using existing catalog (create catalog instead)
+#[allow(clippy::too_many_arguments)]
 pub fn request_and_restore_media(
     worker: &WorkerTask,
     media_id: &MediaId,
@@ -843,13 +844,12 @@ pub fn fast_catalog_restore(
                 let wanted = media_set
                     .media_list()
                     .iter()
-                    .find(|e| {
+                    .any(|e| {
                         match e {
                             None => false,
                             Some(uuid) => uuid == catalog_uuid,
                         }
-                    })
-                    .is_some();
+                    });
 
                 if !wanted {
                     task_log!(worker, "skip catalog because media '{}' not inventarized", catalog_uuid);
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 02/12] tape/changer: clippy fixes

[Dominik Csapak]

fixes:
* unnecessary referencing
* `.map(|v| *v)` => `.copied()`

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/changer/online_status_map.rs | 4 ++--
 src/tape/changer/sg_pt_changer.rs     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/tape/changer/online_status_map.rs b/src/tape/changer/online_status_map.rs
index e30cd17b..324d1fec 100644
--- a/src/tape/changer/online_status_map.rs
+++ b/src/tape/changer/online_status_map.rs
@@ -137,7 +137,7 @@ pub fn update_online_status(state_path: &Path, changer: Option<&str>) -> Result<
 
     for mut changer_config in changers {
         if let Some(changer) = changer {
-            if changer != &changer_config.name {
+            if changer != changer_config.name {
                 continue;
             }
             found_changer = true;
@@ -157,7 +157,7 @@ pub fn update_online_status(state_path: &Path, changer: Option<&str>) -> Result<
     let vtapes: Vec<VirtualTapeDrive> = config.convert_to_typed_array("virtual")?;
     for mut vtape in vtapes {
         if let Some(changer) = changer {
-            if changer != &vtape.name {
+            if changer != vtape.name {
                 continue;
             }
             found_changer = true;
diff --git a/src/tape/changer/sg_pt_changer.rs b/src/tape/changer/sg_pt_changer.rs
index 785fc9ce..b559653c 100644
--- a/src/tape/changer/sg_pt_changer.rs
+++ b/src/tape/changer/sg_pt_changer.rs
@@ -392,7 +392,7 @@ pub fn read_element_status<F: AsRawFd>(file: &mut F) -> Result<MtxStatus, Error>
     for drive in status.drives.iter_mut() {
         if let Some(source_address) = drive.loaded_slot {
             let source_address = source_address as u16;
-            drive.loaded_slot = slot_map.get(&source_address).map(|v| *v);
+            drive.loaded_slot = slot_map.get(&source_address).copied();
         }
     }
 
-- 
2.20.1

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

[Dominik Csapak]

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

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/drive/linux_tape.rs   |  2 +-
 src/tape/drive/mam.rs          | 12 +++++++-----
 src/tape/drive/mod.rs          |  4 +++-
 src/tape/drive/virtual_tape.rs |  2 +-
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/tape/drive/linux_tape.rs b/src/tape/drive/linux_tape.rs
index f8949196..7cb2c61b 100644
--- a/src/tape/drive/linux_tape.rs
+++ b/src/tape/drive/linux_tape.rs
@@ -618,7 +618,7 @@ impl TapeDriver for LinuxTapeHandle {
 
                         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/mam.rs b/src/tape/drive/mam.rs
index cbb377d3..ef47a3d4 100644
--- a/src/tape/drive/mam.rs
+++ b/src/tape/drive/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 5509728c..4d1151ef 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -244,6 +244,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.
@@ -254,7 +256,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 d6b3d0c9..93bf84b4 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -336,7 +336,7 @@ impl TapeDriver for VirtualTapeHandle {
             Some(VirtualTapeStatus { ref mut pos, .. }) => {
 
                 if count <= *pos {
-                    *pos = *pos - count;
+                    *pos -= count;
                 } else {
                     bail!("backward_space_count_files failed: move before BOT");
                 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 04/12] tape/media_*: clippy fixes

[Dominik Csapak]

fixes:
* impl (or derive) Default if struct has 'new()'
* use `or_insert_with`
* combine if branches

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/media_catalog.rs | 10 ++++++----
 src/tape/media_pool.rs    | 16 ++++++----------
 src/tape/media_set.rs     |  4 ++++
 3 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/src/tape/media_catalog.rs b/src/tape/media_catalog.rs
index 04eb50b2..f6e51eea 100644
--- a/src/tape/media_catalog.rs
+++ b/src/tape/media_catalog.rs
@@ -30,6 +30,7 @@ use crate::{
     },
 };
 
+#[derive(Default)]
 pub struct DatastoreContent {
     pub snapshot_index: HashMap<String, u64>, // snapshot => file_nr
     pub chunk_index: HashMap<[u8;32], u64>, // chunk => file_nr
@@ -575,7 +576,7 @@ impl MediaCatalog {
         unsafe { self.pending.write_le_value(entry)?; }
         self.pending.extend(store.as_bytes());
 
-        self.content.entry(store.to_string()).or_insert(DatastoreContent::new());
+        self.content.entry(store.to_string()).or_insert_with(DatastoreContent::new);
 
         self.current_archive = Some((uuid, file_number, store.to_string()));
 
@@ -683,7 +684,7 @@ impl MediaCatalog {
         self.pending.extend(snapshot.as_bytes());
 
         let content = self.content.entry(store.to_string())
-            .or_insert(DatastoreContent::new());
+            .or_insert_with(DatastoreContent::new);
 
         content.snapshot_index.insert(snapshot.to_string(), file_number);
 
@@ -804,7 +805,7 @@ impl MediaCatalog {
                     self.check_start_chunk_archive(file_number)?;
 
                     self.content.entry(store.to_string())
-                        .or_insert(DatastoreContent::new());
+                        .or_insert_with(DatastoreContent::new);
 
                     self.current_archive = Some((uuid, file_number, store.to_string()));
                }
@@ -838,7 +839,7 @@ impl MediaCatalog {
                     self.check_register_snapshot(file_number, snapshot)?;
 
                     let content = self.content.entry(store.to_string())
-                        .or_insert(DatastoreContent::new());
+                        .or_insert_with(DatastoreContent::new);
 
                     content.snapshot_index.insert(snapshot.to_string(), file_number);
 
@@ -879,6 +880,7 @@ impl MediaCatalog {
 /// Media set catalog
 ///
 /// Catalog for multiple media.
+#[derive(Default)]
 pub struct MediaSetCatalog  {
     catalog_list: HashMap<Uuid, MediaCatalog>,
 }
diff --git a/src/tape/media_pool.rs b/src/tape/media_pool.rs
index 3d5eba83..fce4962e 100644
--- a/src/tape/media_pool.rs
+++ b/src/tape/media_pool.rs
@@ -347,13 +347,11 @@ impl MediaPool {
             MediaLocation::Online(name) => {
                 if self.force_media_availability {
                     true
+                } else if let Some(ref changer_name) = self.changer_name {
+                    name == changer_name
                 } else {
-                    if let Some(ref changer_name) = self.changer_name {
-                        name == changer_name
-                    } else {
-                        // a standalone drive cannot use media currently inside a library
-                        false
-                    }
+                    // a standalone drive cannot use media currently inside a library
+                    false
                 }
             }
             MediaLocation::Offline => {
@@ -604,10 +602,8 @@ impl MediaPool {
                     let media_location = media.location();
                     if self.location_is_available(media_location) {
                         last_is_writable = true;
-                    } else {
-                        if let MediaLocation::Vault(vault) = media_location {
-                            bail!("writable media offsite in vault '{}'", vault);
-                        }
+                    } else if let MediaLocation::Vault(vault) = media_location {
+                        bail!("writable media offsite in vault '{}'", vault);
                     }
                 },
                 _ => bail!("unable to use media set - wrong media status {:?}", media.status()),
diff --git a/src/tape/media_set.rs b/src/tape/media_set.rs
index 5568e7f6..858a9e77 100644
--- a/src/tape/media_set.rs
+++ b/src/tape/media_set.rs
@@ -74,3 +74,7 @@ impl MediaSet {
         }
     }
 }
+
+impl Default for MediaSet {
+    fn default() -> Self { Self::new() }
+}
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 05/12] tape/pool_writer: clippy fixes

[Dominik Csapak]

fixes:
* impl (or derive) Default for structs with `new()`
* put complex type in type definition

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/pool_writer/catalog_set.rs         | 1 +
 src/tape/pool_writer/mod.rs                 | 4 +++-
 src/tape/pool_writer/new_chunks_iterator.rs | 4 +++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/tape/pool_writer/catalog_set.rs b/src/tape/pool_writer/catalog_set.rs
index fbca3e97..b3630915 100644
--- a/src/tape/pool_writer/catalog_set.rs
+++ b/src/tape/pool_writer/catalog_set.rs
@@ -12,6 +12,7 @@ use crate::{
 /// Helper to build and query sets of catalogs
 ///
 /// Similar to MediaSetCatalog, but allows to modify the last catalog.
+#[derive(Default)]
 pub struct CatalogSet {
     // read only part
     pub media_set_catalog: MediaSetCatalog,
diff --git a/src/tape/pool_writer/mod.rs b/src/tape/pool_writer/mod.rs
index 05aa52a4..0a79ef3c 100644
--- a/src/tape/pool_writer/mod.rs
+++ b/src/tape/pool_writer/mod.rs
@@ -531,6 +531,8 @@ impl PoolWriter {
     }
 }
 
+type WriteChunkArchiveResult = (Vec<[u8;32]>, Uuid, bool, usize);
+
 /// write up to <max_size> of chunks
 fn write_chunk_archive<'a>(
     _worker: &WorkerTask,
@@ -538,7 +540,7 @@ fn write_chunk_archive<'a>(
     chunk_iter: &mut std::iter::Peekable<NewChunksIterator>,
     store: &str,
     max_size: usize,
-) -> Result<(Vec<[u8;32]>, Uuid, bool, usize), Error> {
+) -> Result<WriteChunkArchiveResult, Error> {
 
     let (mut writer, content_uuid) = ChunkArchiveWriter::new(writer, store, true)?;
 
diff --git a/src/tape/pool_writer/new_chunks_iterator.rs b/src/tape/pool_writer/new_chunks_iterator.rs
index 56491356..55ea407d 100644
--- a/src/tape/pool_writer/new_chunks_iterator.rs
+++ b/src/tape/pool_writer/new_chunks_iterator.rs
@@ -15,12 +15,14 @@ use crate::{
     },
 };
 
+type ChunkReceiver = std::sync::mpsc::Receiver<Result<Option<([u8; 32], DataBlob)>, Error>>;
+
 /// Chunk iterator which use a separate thread to read chunks
 ///
 /// The iterator skips duplicate chunks and chunks already in the
 /// catalog.
 pub struct NewChunksIterator {
-    rx: std::sync::mpsc::Receiver<Result<Option<([u8; 32], DataBlob)>, Error>>,
+    rx: ChunkReceiver,
 }
 
 impl NewChunksIterator {
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 06/12] backup: clippy fixes

[Dominik Csapak]

fixes:
* clone on copy type
* annotate 'wrong_self_convention'

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/backup/key_derivation.rs | 6 +++---
 src/backup/manifest.rs       | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/backup/key_derivation.rs b/src/backup/key_derivation.rs
index 5b46a70c..ece04c4f 100644
--- a/src/backup/key_derivation.rs
+++ b/src/backup/key_derivation.rs
@@ -119,7 +119,7 @@ impl KeyConfig  {
     /// Creates a new, unencrypted key.
     pub fn without_password(raw_key: [u8; 32]) -> Result<Self, Error> {
         // always compute fingerprint
-        let crypt_config = CryptConfig::new(raw_key.clone())?;
+        let crypt_config = CryptConfig::new(raw_key)?;
         let fingerprint = Some(crypt_config.fingerprint());
 
         let created = proxmox::tools::time::epoch_i64();
@@ -186,7 +186,7 @@ impl KeyConfig  {
         let created = proxmox::tools::time::epoch_i64();
 
         // always compute fingerprint
-        let crypt_config = CryptConfig::new(raw_key.clone())?;
+        let crypt_config = CryptConfig::new(*raw_key)?;
         let fingerprint = Some(crypt_config.fingerprint());
 
         Ok(Self {
@@ -257,7 +257,7 @@ impl KeyConfig  {
         let mut result = [0u8; 32];
         result.copy_from_slice(&key);
 
-        let crypt_config = CryptConfig::new(result.clone())?;
+        let crypt_config = CryptConfig::new(result)?;
         let fingerprint = crypt_config.fingerprint();
         if let Some(ref stored_fingerprint) = self.fingerprint {
             if &fingerprint != stored_fingerprint {
diff --git a/src/backup/manifest.rs b/src/backup/manifest.rs
index 47f9cadc..f3a149bd 100644
--- a/src/backup/manifest.rs
+++ b/src/backup/manifest.rs
@@ -148,6 +148,7 @@ impl BackupManifest {
     }
 
     // Generate canonical json
+    #[allow(clippy::wrong_self_convention)]
     fn to_canonical_json(value: &Value) -> Result<Vec<u8>, Error> {
         crate::tools::json::to_canonical_json(value)
     }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 07/12] pxar: clippy fix `or_fun_call`

[Dominik Csapak]

replace ok_or with ok_or_else to do lazy evaluation

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/pxar/extract.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/pxar/extract.rs b/src/pxar/extract.rs
index 32796837..d04ea383 100644
--- a/src/pxar/extract.rs
+++ b/src/pxar/extract.rs
@@ -518,7 +518,7 @@ where
     let root = decoder.open_root().await?;
     let file = root
         .lookup(&path).await?
-        .ok_or(format_err!("error opening '{:?}'", path.as_ref()))?;
+        .ok_or_else(|| format_err!("error opening '{:?}'", path.as_ref()))?;
 
     let mut prefix = PathBuf::new();
     let mut components = file.entry().path().components();
@@ -653,7 +653,7 @@ where
 
     let file = root
         .lookup(&path).await?
-        .ok_or(format_err!("error opening '{:?}'", path.as_ref()))?;
+        .ok_or_else(|| format_err!("error opening '{:?}'", path.as_ref()))?;
 
     recurse_files_extractor(&mut extractor, &mut decoder, file, verbose).await
 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 08/12] bin: clippy fixes

[Dominik Csapak]

fixes:
* `len() < 1` => `is_empty()`
* redundant closure
* unnecessary return

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/bin/docgen.rs | 2 +-
 src/bin/pmt.rs    | 2 +-
 src/bin/pmtx.rs   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/bin/docgen.rs b/src/bin/docgen.rs
index 9df9f33b..1f9657ea 100644
--- a/src/bin/docgen.rs
+++ b/src/bin/docgen.rs
@@ -46,7 +46,7 @@ fn main() -> Result<(), Error> {
 
     let (_prefix, args) = get_args();
 
-    if args.len() < 1 {
+    if args.is_empty() {
         bail!("missing arguments");
     }
 
diff --git a/src/bin/pmt.rs b/src/bin/pmt.rs
index a097df2c..b424b045 100644
--- a/src/bin/pmt.rs
+++ b/src/bin/pmt.rs
@@ -781,7 +781,7 @@ fn st_options(
             list
         }
         Some(false) | None => {
-            options.unwrap_or_else(|| Vec::new())
+            options.unwrap_or_else(Vec::new)
         }
     };
 
diff --git a/src/bin/pmtx.rs b/src/bin/pmtx.rs
index 85114811..aef425b4 100644
--- a/src/bin/pmtx.rs
+++ b/src/bin/pmtx.rs
@@ -262,7 +262,7 @@ fn unload(
 
         if let Some(to_slot) = status.find_free_slot(false) {
             sg_pt_changer::unload(&mut file, to_slot, drivenum)?;
-            return Ok(());
+            Ok(())
         } else {
             bail!("Drive '{}' unload failure - no free slot", drivenum);
         }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 09/12] tape/*: clippy fixes

[Dominik Csapak]

fixes:
* absurd extreme comparisons
* or_fun_call

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/file_formats/multi_volume_writer.rs | 2 +-
 src/tape/inventory.rs                        | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/tape/file_formats/multi_volume_writer.rs b/src/tape/file_formats/multi_volume_writer.rs
index d58285e9..8e0e1d94 100644
--- a/src/tape/file_formats/multi_volume_writer.rs
+++ b/src/tape/file_formats/multi_volume_writer.rs
@@ -72,7 +72,7 @@ impl <'a> TapeWrite for MultiVolumeWriter<'a> {
         }
 
         if self.writer.is_none() {
-            if self.header.part_number >= 255 {
+            if self.header.part_number == 255 {
                 proxmox::io_bail!("multi-volume writer: too many parts");
             }
             self.writer = Some(
diff --git a/src/tape/inventory.rs b/src/tape/inventory.rs
index f9654538..15a51e5e 100644
--- a/src/tape/inventory.rs
+++ b/src/tape/inventory.rs
@@ -792,7 +792,7 @@ pub fn lock_media_set(
     path.push(format!(".media-set-{}", media_set_uuid));
     path.set_extension("lck");
 
-    let timeout = timeout.unwrap_or(Duration::new(10, 0));
+    let timeout = timeout.unwrap_or_else(|| Duration::new(10, 0));
     let file = open_file_locked(&path, timeout, true)?;
     if cfg!(test) {
         // We cannot use chown inside test environment (no permissions)
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 10/12] tools: clippy fixes

[Dominik Csapak]

fixes:
* needless static lifetimes
* unnecessary returns
* `len() == 0` => `is_empty()`

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/fs.rs       |  3 +++
 src/tools/sgutils2.rs | 18 +++++++++---------
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/tools/fs.rs b/src/tools/fs.rs
index 72a530d8..0397113b 100644
--- a/src/tools/fs.rs
+++ b/src/tools/fs.rs
@@ -75,6 +75,9 @@ impl ReadDirEntry {
         self.parent_fd
     }
 
+    /// # Safety
+    ///
+    /// same safety concerns as `std::str::from_utf8_unchecked`
     pub unsafe fn file_name_utf8_unchecked(&self) -> &str {
         std::str::from_utf8_unchecked(self.file_name().to_bytes())
     }
diff --git a/src/tools/sgutils2.rs b/src/tools/sgutils2.rs
index 987d5738..5d7539a2 100644
--- a/src/tools/sgutils2.rs
+++ b/src/tools/sgutils2.rs
@@ -114,7 +114,7 @@ impl SgPt {
 /// Peripheral device type text (see `inquiry` command)
 ///
 /// see [https://en.wikipedia.org/wiki/SCSI_Peripheral_Device_Type]
-pub const PERIPHERAL_DEVICE_TYPE_TEXT: [&'static str; 32] = [
+pub const PERIPHERAL_DEVICE_TYPE_TEXT: [&str; 32] = [
     "Disk Drive",
     "Tape Drive",
     "Printer",
@@ -165,7 +165,7 @@ pub const SENSE_KEY_VOLUME_OVERFLOW: u8 = 0x0d;
 pub const SENSE_KEY_MISCOMPARE: u8      = 0x0e;
 
 /// Sense Key Descriptions
-pub const SENSE_KEY_DESCRIPTIONS: [&'static str; 16] = [
+pub const SENSE_KEY_DESCRIPTIONS: [&str; 16] = [
     "No Sense",
     "Recovered Error",
     "Not Ready",
@@ -447,13 +447,13 @@ impl <'a, F: AsRawFd> SgRaw<'a, F> {
 
         let res_cat = unsafe { get_scsi_pt_result_category(ptvp.as_ptr()) };
         match res_cat {
-            SCSI_PT_RESULT_GOOD => return Ok(()),
+            SCSI_PT_RESULT_GOOD => Ok(()),
             SCSI_PT_RESULT_STATUS => {
                 let status = unsafe { get_scsi_pt_status_response(ptvp.as_ptr()) };
                 if status != 0 {
                     return Err(format_err!("unknown scsi error - status response {}", status).into());
                 }
-                return Ok(());
+                Ok(())
             }
             SCSI_PT_RESULT_SENSE => {
                 if sense_len == 0 {
@@ -489,15 +489,15 @@ impl <'a, F: AsRawFd> SgRaw<'a, F> {
                     }
                 };
 
-                return Err(ScsiError::Sense(sense));
+                Err(ScsiError::Sense(sense))
             }
-            SCSI_PT_RESULT_TRANSPORT_ERR => return Err(format_err!("scsi command failed: transport error").into()),
+            SCSI_PT_RESULT_TRANSPORT_ERR => Err(format_err!("scsi command failed: transport error").into()),
             SCSI_PT_RESULT_OS_ERR => {
                 let errno = unsafe { get_scsi_pt_os_err(ptvp.as_ptr()) };
                 let err = nix::Error::from_errno(nix::errno::Errno::from_i32(errno));
-                return Err(format_err!("scsi command failed with err {}", err).into());
+                Err(format_err!("scsi command failed with err {}", err).into())
             }
-            unknown => return Err(format_err!("scsi command failed: unknown result category {}", unknown).into()),
+            unknown => Err(format_err!("scsi command failed: unknown result category {}", unknown).into()),
         }
     }
 
@@ -540,7 +540,7 @@ impl <'a, F: AsRawFd> SgRaw<'a, F> {
             return Err(format_err!("no valid SCSI command").into());
         }
 
-        if data.len() == 0 {
+        if data.is_empty() {
             return Err(format_err!("got zero-sized input buffer").into());
         }
 
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 11/12] config/tape_encryption_keys: clippy fixes

[Dominik Csapak]

fixes:
* combine if branches
* unnecessary clone

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/config/tape_encryption_keys.rs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/config/tape_encryption_keys.rs b/src/config/tape_encryption_keys.rs
index 42c3184d..db3c9fbf 100644
--- a/src/config/tape_encryption_keys.rs
+++ b/src/config/tape_encryption_keys.rs
@@ -201,17 +201,15 @@ pub fn insert_key(key: [u8;32], key_config: KeyConfig, force: bool) -> Result<()
         None => bail!("missing encryption key fingerprint - internal error"),
     };
 
-    if !force {
-        if config_map.get(&fingerprint).is_some() {
-            bail!("encryption key '{}' already exists.", fingerprint);
-        }
+    if !force && config_map.get(&fingerprint).is_some() {
+        bail!("encryption key '{}' already exists.", fingerprint);
     }
 
     let item = EncryptionKeyInfo::new(key, fingerprint.clone());
     key_map.insert(fingerprint.clone(), item);
     save_keys(key_map)?;
 
-    config_map.insert(fingerprint.clone(), key_config);
+    config_map.insert(fingerprint, key_config);
     save_key_configs(config_map)?;
 
     Ok(())
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 12/12] server/worker_task: clippy fix

[Dominik Csapak]

use '.to_string()' instead of 'format!()' for static str

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/server/worker_task.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 6c5456c9..3e76fad0 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -743,7 +743,7 @@ impl WorkerTask {
 
         let prev_abort = self.abort_requested.swap(true, Ordering::SeqCst);
         if !prev_abort { // log abort one time
-            self.log(format!("received abort request ..."));
+            self.log("received abort request ...".to_string());
         }
         // noitify listeners
         let mut data = self.data.lock().unwrap();
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 00/12] clippy fixes

[Dominik Csapak]

On 4/6/21 9:30 AM, Dietmar Maurer wrote:
> IO am currently rewriting large parts of the tape driver, so
> this is a bad time for clippy fixes...

nvm then, it was just tedious to check my own code with clippy with
those warnings/errors in place. i'll happily send another round
of these later when you're done

> 
>> On 04/06/2021 8:27 AM Dominik Csapak <d.csapak@proxmox.com> wrote:
>>
>>   
>> various clippy fixes all over the place
>> if the separation is not ok, i can send them ofc separated
>> differently
>>
>> (they were not too many of the same type to group by warning,
>> and too spread out to group by file, so i settled for some middle
>> ground)
>>
>> Dominik Csapak (12):
>>    api2/tape: clippy fixes
>>    tape/changer: clippy fixes
>>    tape/drive: clippy fixes
>>    tape/media_*: clippy fixes
>>    tape/pool_writer: clippy fixes
>>    backup: clippy fixes
>>    pxar: clippy fix `or_fun_call`
>>    bin: clippy fixes
>>    tape/*: clippy fixes
>>    tools: clippy fixes
>>    config/tape_encryption_keys: clippy fixes
>>    server/worker_task: clippy fix
>>
>>   src/api2/config/tape_backup_job.rs           |  1 +
>>   src/api2/tape/backup.rs                      |  2 +-
>>   src/api2/tape/media.rs                       | 42 ++++++++++----------
>>   src/api2/tape/restore.rs                     | 12 +++---
>>   src/backup/key_derivation.rs                 |  6 +--
>>   src/backup/manifest.rs                       |  1 +
>>   src/bin/docgen.rs                            |  2 +-
>>   src/bin/pmt.rs                               |  2 +-
>>   src/bin/pmtx.rs                              |  2 +-
>>   src/config/tape_encryption_keys.rs           |  8 ++--
>>   src/pxar/extract.rs                          |  4 +-
>>   src/server/worker_task.rs                    |  2 +-
>>   src/tape/changer/online_status_map.rs        |  4 +-
>>   src/tape/changer/sg_pt_changer.rs            |  2 +-
>>   src/tape/drive/linux_tape.rs                 |  2 +-
>>   src/tape/drive/mam.rs                        | 12 +++---
>>   src/tape/drive/mod.rs                        |  4 +-
>>   src/tape/drive/virtual_tape.rs               |  2 +-
>>   src/tape/file_formats/multi_volume_writer.rs |  2 +-
>>   src/tape/inventory.rs                        |  2 +-
>>   src/tape/media_catalog.rs                    | 10 +++--
>>   src/tape/media_pool.rs                       | 16 +++-----
>>   src/tape/media_set.rs                        |  4 ++
>>   src/tape/pool_writer/catalog_set.rs          |  1 +
>>   src/tape/pool_writer/mod.rs                  |  4 +-
>>   src/tape/pool_writer/new_chunks_iterator.rs  |  4 +-
>>   src/tools/fs.rs                              |  3 ++
>>   src/tools/sgutils2.rs                        | 18 ++++-----
>>   28 files changed, 93 insertions(+), 81 deletions(-)
>>
>> -- 
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup 00/12] clippy fixes

[Dietmar Maurer]

IO am currently rewriting large parts of the tape driver, so
this is a bad time for clippy fixes...

> On 04/06/2021 8:27 AM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> various clippy fixes all over the place
> if the separation is not ok, i can send them ofc separated
> differently
> 
> (they were not too many of the same type to group by warning,
> and too spread out to group by file, so i settled for some middle
> ground)
> 
> Dominik Csapak (12):
>   api2/tape: clippy fixes
>   tape/changer: clippy fixes
>   tape/drive: clippy fixes
>   tape/media_*: clippy fixes
>   tape/pool_writer: clippy fixes
>   backup: clippy fixes
>   pxar: clippy fix `or_fun_call`
>   bin: clippy fixes
>   tape/*: clippy fixes
>   tools: clippy fixes
>   config/tape_encryption_keys: clippy fixes
>   server/worker_task: clippy fix
> 
>  src/api2/config/tape_backup_job.rs           |  1 +
>  src/api2/tape/backup.rs                      |  2 +-
>  src/api2/tape/media.rs                       | 42 ++++++++++----------
>  src/api2/tape/restore.rs                     | 12 +++---
>  src/backup/key_derivation.rs                 |  6 +--
>  src/backup/manifest.rs                       |  1 +
>  src/bin/docgen.rs                            |  2 +-
>  src/bin/pmt.rs                               |  2 +-
>  src/bin/pmtx.rs                              |  2 +-
>  src/config/tape_encryption_keys.rs           |  8 ++--
>  src/pxar/extract.rs                          |  4 +-
>  src/server/worker_task.rs                    |  2 +-
>  src/tape/changer/online_status_map.rs        |  4 +-
>  src/tape/changer/sg_pt_changer.rs            |  2 +-
>  src/tape/drive/linux_tape.rs                 |  2 +-
>  src/tape/drive/mam.rs                        | 12 +++---
>  src/tape/drive/mod.rs                        |  4 +-
>  src/tape/drive/virtual_tape.rs               |  2 +-
>  src/tape/file_formats/multi_volume_writer.rs |  2 +-
>  src/tape/inventory.rs                        |  2 +-
>  src/tape/media_catalog.rs                    | 10 +++--
>  src/tape/media_pool.rs                       | 16 +++-----
>  src/tape/media_set.rs                        |  4 ++
>  src/tape/pool_writer/catalog_set.rs          |  1 +
>  src/tape/pool_writer/mod.rs                  |  4 +-
>  src/tape/pool_writer/new_chunks_iterator.rs  |  4 +-
>  src/tools/fs.rs                              |  3 ++
>  src/tools/sgutils2.rs                        | 18 ++++-----
>  28 files changed, 93 insertions(+), 81 deletions(-)
> 
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel