all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes
@ 2021-04-16 10:28 Dominik Csapak
  2021-04-16 10:28 ` [pbs-devel] [PATCH proxmox-backup v2 01/14] api2/tape: " Dominik Csapak
                   ` (13 more replies)
  0 siblings, 14 replies; 16+ messages in thread
From: Dominik Csapak @ 2021-04-16 10:28 UTC (permalink / raw)
  To: pbs-devel

rebased on master, fixed new errors/warnings that popped up

Dominik Csapak (14):
  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
  bin/proxmox-file-restore: clippy fixes
  bin/proxmox-restore-daemon: clippy fixes

 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/pmtx.rs                               |  2 +-
 src/bin/proxmox-file-restore.rs               |  4 +-
 .../proxmox_file_restore/block_driver_qemu.rs |  2 +-
 src/bin/proxmox_file_restore/qemu_helper.rs   |  4 +-
 src/bin/proxmox_restore_daemon/auth.rs        |  8 ++--
 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/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 +-
 src/tape/file_formats/blocked_reader.rs       |  4 +-
 src/tape/file_formats/chunk_archive.rs        | 12 +++---
 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/cpio.rs                             |  1 +
 src/tools/fs.rs                               |  3 ++
 src/tools/sgutils2.rs                         | 18 ++++----
 35 files changed, 135 insertions(+), 120 deletions(-)

-- 
2.20.1





^ permalink raw reply	[flat|nested] 16+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup v2 09/14] tape/*: clippy fixes
@ 2021-04-19  8:57 Wolfgang Bumiller
  0 siblings, 0 replies; 16+ messages in thread
From: Wolfgang Bumiller @ 2021-04-19  8:57 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Dominik Csapak


> On 04/16/2021 12:29 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> fixes:
> * absurd extreme comparisons
> * or_fun_call
> * unnecessary return
> * collapsible if chain
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/tape/file_formats/blocked_reader.rs      |  4 ++--
>  src/tape/file_formats/chunk_archive.rs       | 12 +++++-------
>  src/tape/file_formats/multi_volume_writer.rs |  2 +-
>  src/tape/inventory.rs                        |  2 +-
>  4 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/src/tape/file_formats/blocked_reader.rs b/src/tape/file_formats/blocked_reader.rs
> index ce2fc9aa..bfdbc2e7 100644
> --- a/src/tape/file_formats/blocked_reader.rs
> +++ b/src/tape/file_formats/blocked_reader.rs
> @@ -118,13 +118,13 @@ impl <R: BlockRead> BlockedReader<R> {
>                  proxmox::io_bail!("detected tape block after block-stream end marker");
>              }
>              Err(BlockReadError::EndOfFile) => {
> -                return Ok(());
> +                Ok(())
>              }
>              Err(BlockReadError::EndOfStream) => {
>                  proxmox::io_bail!("got unexpected end of tape");
>              }
>              Err(BlockReadError::Error(err)) => {
> -                return Err(err);
> +                Err(err)
>              }
>          }
>      }
> diff --git a/src/tape/file_formats/chunk_archive.rs b/src/tape/file_formats/chunk_archive.rs
> index f6b07806..ab4234dd 100644
> --- a/src/tape/file_formats/chunk_archive.rs
> +++ b/src/tape/file_formats/chunk_archive.rs
> @@ -120,13 +120,11 @@ impl <'a> ChunkArchiveWriter<'a> {
>              } else {
>                  self.write_all(&blob_data[start..end])?
>              };
> -            if leom {
> -                if self.close_on_leom {
> -                    let mut writer = self.writer.take().unwrap();
> -                    writer.finish(false)?;
> -                    self.bytes_written = writer.bytes_written();
> -                    return Ok(chunk_is_complete);
> -                }
> +            if leom && self.close_on_leom {
> +                let mut writer = self.writer.take().unwrap();
> +                writer.finish(false)?;
> +                self.bytes_written = writer.bytes_written();
> +                return Ok(chunk_is_complete);
>              }
>              start = end;
>          }
> 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 {

So this caught my attention.

This method increments this to at most 255, which makes sense, given that this is an u8.
(But the incrementation & check could be closer together, be a separate method, or use .checked_add()

But what I'm worried about is the reader side where we have:

    let expect_part_number = self.header.part_number + 1;

This should probably also sanity-check this possible overflow in case of bad/corrupted/fabricated data, no?

So if `255` is a valid number

>                  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));

Oof, why is `Duration::new` not a `const fn` O.o

>      let file = open_file_locked(&path, timeout, true)?;
>      if cfg!(test) {
>          // We cannot use chown inside test environment (no permissions)
> -- 
> 2.20.1




^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-04-19  8:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-16 10:28 [pbs-devel] [PATCH proxmox-backup v2 00/14] various clippy fixes Dominik Csapak
2021-04-16 10:28 ` [pbs-devel] [PATCH proxmox-backup v2 01/14] api2/tape: " Dominik Csapak
2021-04-16 10:28 ` [pbs-devel] [PATCH proxmox-backup v2 02/14] tape/changer: " Dominik Csapak
2021-04-16 10:28 ` [pbs-devel] [PATCH proxmox-backup v2 03/14] tape/drive: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 04/14] tape/media_*: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 05/14] tape/pool_writer: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 06/14] backup: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 07/14] pxar: clippy fix `or_fun_call` Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 08/14] bin: clippy fixes Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 09/14] tape/*: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 10/14] tools: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 11/14] config/tape_encryption_keys: " Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 12/14] server/worker_task: clippy fix Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 13/14] bin/proxmox-file-restore: clippy fixes Dominik Csapak
2021-04-16 10:29 ` [pbs-devel] [PATCH proxmox-backup v2 14/14] bin/proxmox-restore-daemon: " Dominik Csapak
2021-04-19  8:57 [pbs-devel] [PATCH proxmox-backup v2 09/14] tape/*: " Wolfgang Bumiller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal