* [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 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