[pbs-devel] [PATCH backup v4 1/2] pxar-fuse: use ReplyBufState::is_full() when possible

[pbs-devel] [PATCH backup v4 1/2] pxar-fuse: use ReplyBufState::is_full() when possible

[Maximiliano Sandoval]

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
Differences from vN-1:
 - Separate into two commits
 - Leave unrelated changes for a follow up patch

 pbs-pxar-fuse/src/lib.rs | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/pbs-pxar-fuse/src/lib.rs b/pbs-pxar-fuse/src/lib.rs
index 98d28c57..bf196b6c 100644
--- a/pbs-pxar-fuse/src/lib.rs
+++ b/pbs-pxar-fuse/src/lib.rs
@@ -525,9 +525,11 @@ impl SessionImpl {
             let file = file?.decode_entry().await?;
             let stat = to_stat(to_inode(&file), &file)?;
             let name = file.file_name();
-            match request.add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)? {
-                ReplyBufState::Ok => (),
-                ReplyBufState::Full => return Ok(lookups),
+            if request
+                .add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)?
+                .is_full()
+            {
+                return Ok(lookups);
             }
             lookups.push(self.make_lookup(request.inode, stat.st_ino, &file)?);
         }
@@ -537,9 +539,11 @@ impl SessionImpl {
             let file = dir.lookup_self().await?;
             let stat = to_stat(to_inode(&file), &file)?;
             let name = OsStr::new(".");
-            match request.add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)? {
-                ReplyBufState::Ok => (),
-                ReplyBufState::Full => return Ok(lookups),
+            if request
+                .add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)?
+                .is_full()
+            {
+                return Ok(lookups);
             }
             lookups.push(LookupRef::clone(&dir_lookup));
         }
@@ -551,9 +555,11 @@ impl SessionImpl {
             let file = parent_dir.lookup_self().await?;
             let stat = to_stat(to_inode(&file), &file)?;
             let name = OsStr::new("..");
-            match request.add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)? {
-                ReplyBufState::Ok => (),
-                ReplyBufState::Full => return Ok(lookups),
+            if request
+                .add_entry(name, &stat, next, 1, f64::MAX, f64::MAX)?
+                .is_full()
+            {
+                return Ok(lookups);
             }
             lookups.push(lookup);
         }
@@ -619,9 +625,8 @@ impl SessionImpl {
         let xattrs = self.listxattrs(request.inode).await?;
 
         for entry in xattrs {
-            match request.add_c_string(entry.name()) {
-                ReplyBufState::Ok => (),
-                ReplyBufState::Full => return Ok(ReplyBufState::Full),
+            if request.add_c_string(entry.name()).is_full() {
+                return Ok(ReplyBufState::Full);
             }
         }
 
-- 
2.39.2

[pbs-devel] [PATCH backup v4 2/2] api: use if-let pattern for error-only handling

[Maximiliano Sandoval]

It is more readable than using match. We also inline variables in
eprintln!.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/pxar/dir_stack.rs            |  9 +++------
 src/api2/access/user.rs                     | 20 ++++----------------
 src/bin/proxmox-daily-update.rs             |  7 ++-----
 src/tape/pool_writer/new_chunks_iterator.rs |  9 +++------
 src/tools/parallel_handler.rs               | 11 ++++-------
 src/traffic_control_cache.rs                |  7 ++-----
 6 files changed, 18 insertions(+), 45 deletions(-)

diff --git a/pbs-client/src/pxar/dir_stack.rs b/pbs-client/src/pxar/dir_stack.rs
index 43cbee1d..616d7545 100644
--- a/pbs-client/src/pxar/dir_stack.rs
+++ b/pbs-client/src/pxar/dir_stack.rs
@@ -40,16 +40,13 @@ impl PxarDir {
         parent: RawFd,
         allow_existing_dirs: bool,
     ) -> Result<BorrowedFd, Error> {
-        match mkdirat(
+        if let Err(err) = mkdirat(
             parent,
             self.file_name.as_os_str(),
             perms_from_metadata(&self.metadata)?,
         ) {
-            Ok(()) => (),
-            Err(err) => {
-                if !(allow_existing_dirs && err.already_exists()) {
-                    return Err(err.into());
-                }
+            if !(allow_existing_dirs && err.already_exists()) {
+                return Err(err.into());
             }
         }
 
diff --git a/src/api2/access/user.rs b/src/api2/access/user.rs
index 118838ce..a0be6111 100644
--- a/src/api2/access/user.rs
+++ b/src/api2/access/user.rs
@@ -381,28 +381,16 @@ pub fn delete_user(userid: Userid, digest: Option<String>) -> Result<(), Error>
     pbs_config::user::save_config(&config)?;
 
     let authenticator = crate::auth::lookup_authenticator(userid.realm())?;
-    match authenticator.remove_password(userid.name()) {
-        Ok(()) => {}
-        Err(err) => {
-            eprintln!(
-                "error removing password after deleting user {:?}: {}",
-                userid, err
-            );
-        }
+    if let Err(err) = authenticator.remove_password(userid.name()) {
+        eprintln!("error removing password after deleting user {userid:?}: {err}",);
     }
 
-    match crate::config::tfa::read().and_then(|mut cfg| {
+    if let Err(err) = crate::config::tfa::read().and_then(|mut cfg| {
         let _: proxmox_tfa::api::NeedsSaving =
             cfg.remove_user(&crate::config::tfa::UserAccess, userid.as_str())?;
         crate::config::tfa::write(&cfg)
     }) {
-        Ok(()) => (),
-        Err(err) => {
-            eprintln!(
-                "error updating TFA config after deleting user {:?}: {}",
-                userid, err
-            );
-        }
+        eprintln!("error updating TFA config after deleting user {userid:?} {err}",);
     }
 
     Ok(())
diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-update.rs
index ae3744c5..c22609c5 100644
--- a/src/bin/proxmox-daily-update.rs
+++ b/src/bin/proxmox-daily-update.rs
@@ -55,11 +55,8 @@ async fn do_update(rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> {
         _ => unreachable!(),
     };
 
-    match check_acme_certificates(rpcenv).await {
-        Ok(()) => (),
-        Err(err) => {
-            log::error!("error checking certificates: {}", err);
-        }
+    if let Err(err) = check_acme_certificates(rpcenv).await {
+        log::error!("error checking certificates: {err}");
     }
 
     // TODO: cleanup tasks like in PVE?
diff --git a/src/tape/pool_writer/new_chunks_iterator.rs b/src/tape/pool_writer/new_chunks_iterator.rs
index ae75b7b1..1454b33d 100644
--- a/src/tape/pool_writer/new_chunks_iterator.rs
+++ b/src/tape/pool_writer/new_chunks_iterator.rs
@@ -57,12 +57,9 @@ impl NewChunksIterator {
 
                     let blob = datastore.load_chunk(&digest)?;
                     //println!("LOAD CHUNK {}", hex::encode(&digest));
-                    match tx.send(Ok(Some((digest, blob)))) {
-                        Ok(()) => {}
-                        Err(err) => {
-                            eprintln!("could not send chunk to reader thread: {}", err);
-                            break;
-                        }
+                    if let Err(err) = tx.send(Ok(Some((digest, blob)))) {
+                        eprintln!("could not send chunk to reader thread: {err}");
+                        break;
                     }
 
                     chunk_index.insert(digest);
diff --git a/src/tools/parallel_handler.rs b/src/tools/parallel_handler.rs
index c4316ad0..17f70179 100644
--- a/src/tools/parallel_handler.rs
+++ b/src/tools/parallel_handler.rs
@@ -80,13 +80,10 @@ impl<I: Send + 'static> ParallelHandler<I> {
                             Ok(data) => data,
                             Err(_) => return,
                         };
-                        match (handler_fn)(data) {
-                            Ok(()) => (),
-                            Err(err) => {
-                                let mut guard = abort.lock().unwrap();
-                                if guard.is_none() {
-                                    *guard = Some(err.to_string());
-                                }
+                        if let Err(err) = (handler_fn)(data) {
+                            let mut guard = abort.lock().unwrap();
+                            if guard.is_none() {
+                                *guard = Some(err.to_string());
                             }
                         }
                     })
diff --git a/src/traffic_control_cache.rs b/src/traffic_control_cache.rs
index 2e097d70..4c3bccee 100644
--- a/src/traffic_control_cache.rs
+++ b/src/traffic_control_cache.rs
@@ -164,11 +164,8 @@ impl TrafficControlCache {
         self.last_traffic_control_generation = traffic_control_generation;
         self.last_update = now;
 
-        match self.reload_impl() {
-            Ok(()) => (),
-            Err(err) => {
-                log::error!("TrafficControlCache::reload failed -> {}", err);
-            }
+        if let Err(err) = self.reload_impl() {
+            log::error!("TrafficControlCache::reload failed -> {err}");
         }
     }
 
-- 
2.39.2

[pbs-devel] applied-series: [PATCH backup v4 1/2] pxar-fuse: use ReplyBufState::is_full() when possible

[Thomas Lamprecht]

Am 14/02/2024 um 09:23 schrieb Maximiliano Sandoval:
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> Differences from vN-1:
>  - Separate into two commits
>  - Leave unrelated changes for a follow up patch
> 
>  pbs-pxar-fuse/src/lib.rs | 29 +++++++++++++++++------------
>  1 file changed, 17 insertions(+), 12 deletions(-)
> 
>

applied, thanks!