* [pbs-devel] [PATCH backup v3 1/2] backup-proxy: avoid block in if condition
@ 2024-02-13 12:43 Maximiliano Sandoval
2024-02-13 12:43 ` [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling Maximiliano Sandoval
2024-02-13 13:58 ` [pbs-devel] applied: [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Fabian Grünbichler
0 siblings, 2 replies; 5+ messages in thread
From: Maximiliano Sandoval @ 2024-02-13 12:43 UTC (permalink / raw)
To: pbs-devel
Fixes the clippy lint:
```
warning: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
--> src/bin/proxmox-backup-proxy.rs:874:58
|
874 | let stats = match tokio::task::spawn_blocking(|| {
| __________________________________________________________^
875 | | let hoststats = collect_host_stats_sync();
876 | | let (hostdisk, datastores) = collect_disk_stats_sync();
877 | | Arc::new((hoststats, hostdisk, datastores))
878 | | })
| |_________^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
= note: `#[warn(clippy::blocks_in_conditions)]` on by default
```
Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
Differences from v2:
- Rename stats_res to stats_fut
- Port other uses of single branched `match` to if-let
src/bin/proxmox-backup-proxy.rs | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index 9c49026b..a2c48f16 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -871,13 +871,12 @@ async fn run_stat_generator() {
loop {
let delay_target = Instant::now() + Duration::from_secs(10);
- let stats = match tokio::task::spawn_blocking(|| {
+ let stats_fut = tokio::task::spawn_blocking(|| {
let hoststats = collect_host_stats_sync();
let (hostdisk, datastores) = collect_disk_stats_sync();
Arc::new((hoststats, hostdisk, datastores))
- })
- .await
- {
+ });
+ let stats = match stats_fut.await {
Ok(res) => res,
Err(err) => {
log::error!("collecting host stats panicked: {err}");
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling
2024-02-13 12:43 [pbs-devel] [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Maximiliano Sandoval
@ 2024-02-13 12:43 ` Maximiliano Sandoval
2024-02-13 14:04 ` Fabian Grünbichler
2024-02-13 13:58 ` [pbs-devel] applied: [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Fabian Grünbichler
1 sibling, 1 reply; 5+ messages in thread
From: Maximiliano Sandoval @ 2024-02-13 12:43 UTC (permalink / raw)
To: pbs-devel
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 ++--
pbs-pxar-fuse/src/lib.rs | 47 ++++++++++++---------
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 +--
7 files changed, 45 insertions(+), 65 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/pbs-pxar-fuse/src/lib.rs b/pbs-pxar-fuse/src/lib.rs
index 98d28c57..5c2b6b49 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);
}
@@ -618,24 +624,25 @@ impl SessionImpl {
) -> Result<ReplyBufState, Error> {
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 xattrs
+ .iter()
+ .any(|entry| request.add_c_string(entry.name()).is_full())
+ {
+ Ok(ReplyBufState::Full)
+ } else {
+ Ok(ReplyBufState::Ok)
}
-
- Ok(ReplyBufState::Ok)
}
async fn getxattr(&self, inode: u64, xattr: &OsStr) -> Result<pxar::format::XAttr, Error> {
// TODO: pxar::Accessor could probably get a more optimized method to fetch a specific
// xattr for an entry...
let xattrs = self.listxattrs(inode).await?;
- for entry in xattrs {
- if entry.name().to_bytes() == xattr.as_bytes() {
- return Ok(entry);
- }
+ if xattrs
+ .iter()
+ .any(|entry| request.add_c_string(entry.name()).is_full())
+ {
+ return Ok(entry);
}
io_return!(libc::ENODATA);
}
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [pbs-devel] applied: [PATCH backup v3 1/2] backup-proxy: avoid block in if condition
2024-02-13 12:43 [pbs-devel] [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Maximiliano Sandoval
2024-02-13 12:43 ` [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling Maximiliano Sandoval
@ 2024-02-13 13:58 ` Fabian Grünbichler
1 sibling, 0 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2024-02-13 13:58 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On February 13, 2024 1:43 pm, Maximiliano Sandoval wrote:
> Fixes the clippy lint:
>
> ```
> warning: in a `match` scrutinee, avoid complex blocks or closures with blocks; instead, move the block or closure higher and bind it with a `let`
> --> src/bin/proxmox-backup-proxy.rs:874:58
> |
> 874 | let stats = match tokio::task::spawn_blocking(|| {
> | __________________________________________________________^
> 875 | | let hoststats = collect_host_stats_sync();
> 876 | | let (hostdisk, datastores) = collect_disk_stats_sync();
> 877 | | Arc::new((hoststats, hostdisk, datastores))
> 878 | | })
> | |_________^
> |
> = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_conditions
> = note: `#[warn(clippy::blocks_in_conditions)]` on by default
> ```
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> Differences from v2:
> - Rename stats_res to stats_fut
> - Port other uses of single branched `match` to if-let
>
> src/bin/proxmox-backup-proxy.rs | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 9c49026b..a2c48f16 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -871,13 +871,12 @@ async fn run_stat_generator() {
> loop {
> let delay_target = Instant::now() + Duration::from_secs(10);
>
> - let stats = match tokio::task::spawn_blocking(|| {
> + let stats_fut = tokio::task::spawn_blocking(|| {
> let hoststats = collect_host_stats_sync();
> let (hostdisk, datastores) = collect_disk_stats_sync();
> Arc::new((hoststats, hostdisk, datastores))
> - })
> - .await
> - {
> + });
> + let stats = match stats_fut.await {
> Ok(res) => res,
> Err(err) => {
> log::error!("collecting host stats panicked: {err}");
> --
> 2.39.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling
2024-02-13 12:43 ` [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling Maximiliano Sandoval
@ 2024-02-13 14:04 ` Fabian Grünbichler
2024-02-13 15:49 ` Maximiliano Sandoval
0 siblings, 1 reply; 5+ messages in thread
From: Fabian Grünbichler @ 2024-02-13 14:04 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
On February 13, 2024 1:43 pm, Maximiliano Sandoval wrote:
> It is more readable than using match. We also inline variables in
> eprintln!.
this does a lot more than what it says here..
>
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> pbs-client/src/pxar/dir_stack.rs | 9 ++--
> pbs-pxar-fuse/src/lib.rs | 47 ++++++++++++---------
> 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 +--
> 7 files changed, 45 insertions(+), 65 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());
fine
> }
> }
>
> diff --git a/pbs-pxar-fuse/src/lib.rs b/pbs-pxar-fuse/src/lib.rs
> index 98d28c57..5c2b6b49 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);
not error handling, but semantically equivalent to if-let, okay..
> }
> 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);
same
> }
> 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);
same
> }
> lookups.push(lookup);
> }
> @@ -618,24 +624,25 @@ impl SessionImpl {
> ) -> Result<ReplyBufState, Error> {
> 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 xattrs
> + .iter()
> + .any(|entry| request.add_c_string(entry.name()).is_full())
> + {
> + Ok(ReplyBufState::Full)
> + } else {
> + Ok(ReplyBufState::Ok)
this is not an if-let at all (and also not error handling ;)), even
though the end result is of course the same..
> }
> -
> - Ok(ReplyBufState::Ok)
> }
>
> async fn getxattr(&self, inode: u64, xattr: &OsStr) -> Result<pxar::format::XAttr, Error> {
> // TODO: pxar::Accessor could probably get a more optimized method to fetch a specific
> // xattr for an entry...
> let xattrs = self.listxattrs(inode).await?;
> - for entry in xattrs {
> - if entry.name().to_bytes() == xattr.as_bytes() {
> - return Ok(entry);
> - }
> + if xattrs
> + .iter()
> + .any(|entry| request.add_c_string(entry.name()).is_full())
> + {
> + return Ok(entry);
but this here is an obvious copy paste error that doesn't even compile!
please check patches before sending, even if they are supposedly trivial
style fixes!
> }
> io_return!(libc::ENODATA);
> }
> 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}",);
this one seems fine
> }
>
> - 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}",);
this one as well
> }
>
> 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}");
okay as well
> }
>
> // 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;
okay
> }
>
> 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());
okay
> }
> }
> })
> 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}");
okay
> }
> }
>
> --
> 2.39.2
>
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling
2024-02-13 14:04 ` Fabian Grünbichler
@ 2024-02-13 15:49 ` Maximiliano Sandoval
0 siblings, 0 replies; 5+ messages in thread
From: Maximiliano Sandoval @ 2024-02-13 15:49 UTC (permalink / raw)
To: Proxmox Backup Server development discussion
Fabian Grünbichler <f.gruenbichler@proxmox.com> writes:
>> async fn getxattr(&self, inode: u64, xattr: &OsStr) -> Result<pxar::format::XAttr, Error> {
>> // TODO: pxar::Accessor could probably get a more optimized method to fetch a specific
>> // xattr for an entry...
>> let xattrs = self.listxattrs(inode).await?;
>> - for entry in xattrs {
>> - if entry.name().to_bytes() == xattr.as_bytes() {
>> - return Ok(entry);
>> - }
>> + if xattrs
>> + .iter()
>> + .any(|entry| request.add_c_string(entry.name()).is_full())
>> + {
>> + return Ok(entry);
>
> but this here is an obvious copy paste error that doesn't even compile!
> please check patches before sending, even if they are supposedly trivial
> style fixes!
My bad, I thought `cargo build` on the root was enough to test this
change. Will sent a vN tomorrow splitting the commits, and fixing this.
--
Maximiliano
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-02-13 16:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-13 12:43 [pbs-devel] [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Maximiliano Sandoval
2024-02-13 12:43 ` [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling Maximiliano Sandoval
2024-02-13 14:04 ` Fabian Grünbichler
2024-02-13 15:49 ` Maximiliano Sandoval
2024-02-13 13:58 ` [pbs-devel] applied: [PATCH backup v3 1/2] backup-proxy: avoid block in if condition Fabian Grünbichler
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox