From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D0CEA91100 for ; Tue, 13 Feb 2024 15:05:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B4F9B36EC2 for ; Tue, 13 Feb 2024 15:04:58 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Tue, 13 Feb 2024 15:04:57 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A590647CA3 for ; Tue, 13 Feb 2024 15:04:57 +0100 (CET) Date: Tue, 13 Feb 2024 15:04:49 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20240213124329.411737-1-m.sandoval@proxmox.com> <20240213124329.411737-2-m.sandoval@proxmox.com> In-Reply-To: <20240213124329.411737-2-m.sandoval@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1707832707.8atxyms769.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL -0.084 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment POISEN_SPAM_PILL 0.1 Meta: its spam POISEN_SPAM_PILL_2 0.1 random spam to be learned in bayes POISEN_SPAM_PILL_4 0.1 random spam to be learned in bayes SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH backup v3 2/2] api: use if-let pattern for error-only handling X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Feb 2024 14:05:28 -0000 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.. >=20 > Signed-off-by: Maximiliano Sandoval > --- > 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(-) >=20 > diff --git a/pbs-client/src/pxar/dir_stack.rs b/pbs-client/src/pxar/dir_s= tack.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 { > - match mkdirat( > + if let Err(err) =3D mkdirat( > parent, > self.file_name.as_os_str(), > perms_from_metadata(&self.metadata)?, > ) { > - Ok(()) =3D> (), > - Err(err) =3D> { > - if !(allow_existing_dirs && err.already_exists()) { > - return Err(err.into()); > - } > + if !(allow_existing_dirs && err.already_exists()) { > + return Err(err.into()); fine > } > } > =20 > 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 =3D file?.decode_entry().await?; > let stat =3D to_stat(to_inode(&file), &file)?; > let name =3D file.file_name(); > - match request.add_entry(name, &stat, next, 1, f64::MAX, f64:= :MAX)? { > - ReplyBufState::Ok =3D> (), > - ReplyBufState::Full =3D> 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, &f= ile)?); > } > @@ -537,9 +539,11 @@ impl SessionImpl { > let file =3D dir.lookup_self().await?; > let stat =3D to_stat(to_inode(&file), &file)?; > let name =3D OsStr::new("."); > - match request.add_entry(name, &stat, next, 1, f64::MAX, f64:= :MAX)? { > - ReplyBufState::Ok =3D> (), > - ReplyBufState::Full =3D> 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 =3D parent_dir.lookup_self().await?; > let stat =3D to_stat(to_inode(&file), &file)?; > let name =3D OsStr::new(".."); > - match request.add_entry(name, &stat, next, 1, f64::MAX, f64:= :MAX)? { > - ReplyBufState::Ok =3D> (), > - ReplyBufState::Full =3D> 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 { > let xattrs =3D self.listxattrs(request.inode).await?; > =20 > - for entry in xattrs { > - match request.add_c_string(entry.name()) { > - ReplyBufState::Ok =3D> (), > - ReplyBufState::Full =3D> 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) > } > =20 > async fn getxattr(&self, inode: u64, xattr: &OsStr) -> Result { > // TODO: pxar::Accessor could probably get a more optimized meth= od to fetch a specific > // xattr for an entry... > let xattrs =3D self.listxattrs(inode).await?; > - for entry in xattrs { > - if entry.name().to_bytes() =3D=3D 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) -> Result<(), Error> > pbs_config::user::save_config(&config)?; > =20 > let authenticator =3D crate::auth::lookup_authenticator(userid.realm= ())?; > - match authenticator.remove_password(userid.name()) { > - Ok(()) =3D> {} > - Err(err) =3D> { > - eprintln!( > - "error removing password after deleting user {:?}: {}", > - userid, err > - ); > - } > + if let Err(err) =3D authenticator.remove_password(userid.name()) { > + eprintln!("error removing password after deleting user {userid:?= }: {err}",); this one seems fine > } > =20 > - match crate::config::tfa::read().and_then(|mut cfg| { > + if let Err(err) =3D crate::config::tfa::read().and_then(|mut cfg| { > let _: proxmox_tfa::api::NeedsSaving =3D > cfg.remove_user(&crate::config::tfa::UserAccess, userid.as_s= tr())?; > crate::config::tfa::write(&cfg) > }) { > - Ok(()) =3D> (), > - Err(err) =3D> { > - eprintln!( > - "error updating TFA config after deleting user {:?}: {}"= , > - userid, err > - ); > - } > + eprintln!("error updating TFA config after deleting user {userid= :?} {err}",); this one as well > } > =20 > Ok(()) > diff --git a/src/bin/proxmox-daily-update.rs b/src/bin/proxmox-daily-upda= te.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> { > _ =3D> unreachable!(), > }; > =20 > - match check_acme_certificates(rpcenv).await { > - Ok(()) =3D> (), > - Err(err) =3D> { > - log::error!("error checking certificates: {}", err); > - } > + if let Err(err) =3D check_acme_certificates(rpcenv).await { > + log::error!("error checking certificates: {err}"); okay as well > } > =20 > // 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 { > =20 > let blob =3D datastore.load_chunk(&digest)?; > //println!("LOAD CHUNK {}", hex::encode(&digest)); > - match tx.send(Ok(Some((digest, blob)))) { > - Ok(()) =3D> {} > - Err(err) =3D> { > - eprintln!("could not send chunk to reader th= read: {}", err); > - break; > - } > + if let Err(err) =3D tx.send(Ok(Some((digest, blob)))= ) { > + eprintln!("could not send chunk to reader thread= : {err}"); > + break; okay > } > =20 > chunk_index.insert(digest); > diff --git a/src/tools/parallel_handler.rs b/src/tools/parallel_handler.r= s > index c4316ad0..17f70179 100644 > --- a/src/tools/parallel_handler.rs > +++ b/src/tools/parallel_handler.rs > @@ -80,13 +80,10 @@ impl ParallelHandler { > Ok(data) =3D> data, > Err(_) =3D> return, > }; > - match (handler_fn)(data) { > - Ok(()) =3D> (), > - Err(err) =3D> { > - let mut guard =3D abort.lock().unwrap(); > - if guard.is_none() { > - *guard =3D Some(err.to_string()); > - } > + if let Err(err) =3D (handler_fn)(data) { > + let mut guard =3D abort.lock().unwrap(); > + if guard.is_none() { > + *guard =3D 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 =3D traffic_control_generat= ion; > self.last_update =3D now; > =20 > - match self.reload_impl() { > - Ok(()) =3D> (), > - Err(err) =3D> { > - log::error!("TrafficControlCache::reload failed -> {}", = err); > - } > + if let Err(err) =3D self.reload_impl() { > + log::error!("TrafficControlCache::reload failed -> {err}"); okay > } > } > =20 > --=20 > 2.39.2 >=20 >=20 >=20 > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >=20 >=20 >=20