From 6dbcdcd784f79e69d7656cb270e8cd63a3cf7677 Mon Sep 17 00:00:00 2001 From: Gwen Lg Date: Sat, 13 Dec 2025 22:56:24 +0100 Subject: [PATCH] style: improve use of std api - replace get(0) by first() lint message: accessing first element with `objs.get(0)` help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#get_first - use io::Error::other method available since Rust 1.87 lint message: this can be `std::io::Error::other(_)` help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#io_other_error - implement From<_> instead of Into<_> lint message: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true help: `impl From for Foreign` is allowed by the orphan rules, for more information see https://doc.rust-lang.org/reference/items/implementations.html#trait-implementation-coherence help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#from_over_into - use next_back instead of rev + next lint message: manual backwards iteration help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#manual_next_back - use saturating_sub instead of manual implement it lint message: manual arithmetic check found help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#implicit_saturating_sub - add Default implementation for Checksummer use derived Default ans use it in new, as new set all value to None lint message: you should consider adding a `Default` implementation for `Checksummer` help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#new_without_default - use `any` instead of `find` + `is_some` lint message: called `is_some()` after searching an `Iterator` with `find` help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#search_is_some - replace `and_then` + `Some(_)` with `map` lint message: using `Option.and_then(|x| Some(y))`, which is more succinctly expressed as `map(|x| y)` help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#bind_instead_of_map - replace `skip_while` + `next` with `find` lint message: called `skip_while(

).next()` on an `Iterator` help: this is more succinctly expressed by calling `.find(!

)` instead help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#skip_while_next - use `matches!` instead of manual implementation lint message: match expression looks like `matches!` macro help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#match_like_matches_macro - use `keys` and `values methods insteads of iterating and ignoring either the keys or values. help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#iter_kv_map --- src/api/admin/layout.rs | 12 ++++++------ src/api/common/signature/checksum.rs | 10 ++-------- src/api/s3/delete.rs | 2 +- src/api/s3/encryption.rs | 13 +++---------- src/api/s3/get.rs | 24 ++++++------------------ src/api/s3/list.rs | 4 ++-- src/garage/cli/remote/bucket.rs | 7 +------ src/net/client.rs | 3 +-- src/net/message.rs | 5 +---- src/net/recv.rs | 7 ++----- src/rpc/layout/helper.rs | 6 +++--- src/rpc/layout/manager.rs | 2 +- src/rpc/system.rs | 4 ++-- 13 files changed, 31 insertions(+), 68 deletions(-) diff --git a/src/api/admin/layout.rs b/src/api/admin/layout.rs index 7f9f5412..6502f779 100644 --- a/src/api/admin/layout.rs +++ b/src/api/admin/layout.rs @@ -380,9 +380,9 @@ impl From for ZoneRedundancy { } } -impl Into for ZoneRedundancy { - fn into(self) -> layout::ZoneRedundancy { - match self { +impl From for layout::ZoneRedundancy { + fn from(val: ZoneRedundancy) -> Self { + match val { ZoneRedundancy::Maximum => layout::ZoneRedundancy::Maximum, ZoneRedundancy::AtLeast(x) => layout::ZoneRedundancy::AtLeast(x), } @@ -397,10 +397,10 @@ impl From for LayoutParameters { } } -impl Into for LayoutParameters { - fn into(self) -> layout::LayoutParameters { +impl From for layout::LayoutParameters { + fn from(val: LayoutParameters) -> Self { layout::LayoutParameters { - zone_redundancy: self.zone_redundancy.into(), + zone_redundancy: val.zone_redundancy.into(), } } } diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 09eae74c..4ae6a18b 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -63,6 +63,7 @@ pub struct ExpectedChecksums { pub extra: Option, } +#[derive(Default)] pub struct Checksummer { pub crc32: Option, pub crc32c: Option, @@ -84,14 +85,7 @@ pub struct Checksums { impl Checksummer { pub fn new() -> Self { - Self { - crc32: None, - crc32c: None, - crc64nvme: None, - md5: None, - sha1: None, - sha256: None, - } + Default::default() } pub fn init(expected: &ExpectedChecksums, add_md5: bool) -> Self { diff --git a/src/api/s3/delete.rs b/src/api/s3/delete.rs index d785b9d8..4b705ad8 100644 --- a/src/api/s3/delete.rs +++ b/src/api/s3/delete.rs @@ -29,7 +29,7 @@ async fn handle_delete_internal(ctx: &ReqCtx, key: &str) -> Result<(Uuid, Uuid), .iter() .rev() .find(|v| !matches!(&v.state, ObjectVersionState::Aborted)) - .or_else(|| object.versions().iter().rev().next()); + .or_else(|| object.versions().iter().next_back()); let deleted_version = match deleted_version { Some(dv) => dv.uuid, None => { diff --git a/src/api/s3/encryption.rs b/src/api/s3/encryption.rs index 0c404e27..ca140af9 100644 --- a/src/api/s3/encryption.rs +++ b/src/api/s3/encryption.rs @@ -94,10 +94,7 @@ impl EncryptionParams { // data blocks are reused as-is. Since Garage v2, we are using // object-specific encryption keys, so we know that if both source // and destination are encrypted, it can't be with the same key. - match (a, b) { - (Self::Plaintext, Self::Plaintext) => true, - _ => false, - } + matches!((a, b), (Self::Plaintext, Self::Plaintext)) } pub fn new_from_headers( @@ -587,8 +584,7 @@ impl Stream for DecryptStream { if matches!(this.state, DecryptStreamState::Done) { if !this.buf.is_empty() { - return Poll::Ready(Some(Err(std::io::Error::new( - std::io::ErrorKind::Other, + return Poll::Ready(Some(Err(std::io::Error::other( "Decrypt: unexpected bytes after last encrypted chunk", )))); } @@ -622,10 +618,7 @@ impl Stream for DecryptStream { match res { Ok(bytes) if bytes.is_empty() => Poll::Ready(None), Ok(bytes) => Poll::Ready(Some(Ok(bytes.into()))), - Err(_) => Poll::Ready(Some(Err(std::io::Error::new( - std::io::ErrorKind::Other, - "Decryption failed", - )))), + Err(_) => Poll::Ready(Some(Err(std::io::Error::other("Decryption failed")))), } } } diff --git a/src/api/s3/get.rs b/src/api/s3/get.rs index bf2e68c7..5a427a87 100644 --- a/src/api/s3/get.rs +++ b/src/api/s3/get.rs @@ -400,15 +400,10 @@ async fn handle_get_full( overrides: GetObjectOverrides, checksum_mode: ChecksumMode, ) -> Result, Error> { - let mut resp_builder = object_headers( - version, - version_meta, - meta_inner, - encryption, - checksum_mode, - ) - .header(CONTENT_LENGTH, format!("{}", version_meta.size)) - .status(StatusCode::OK); + let mut resp_builder = + object_headers(version, version_meta, meta_inner, encryption, checksum_mode) + .header(CONTENT_LENGTH, format!("{}", version_meta.size)) + .status(StatusCode::OK); getobject_override_headers(overrides, &mut resp_builder)?; let stream = full_object_byte_stream(garage, version, version_data, encryption); @@ -708,11 +703,7 @@ fn body_from_blocks_range( Some(None) } else { // The chunk has an intersection with the requested range - let start_in_chunk = if *chunk_offset > begin { - 0 - } else { - begin - *chunk_offset - }; + let start_in_chunk = begin.saturating_sub(*chunk_offset); let end_in_chunk = if *chunk_offset + chunk_len < end { chunk_len } else { @@ -773,10 +764,7 @@ fn error_stream_item(e: E) -> ByteStream { } fn std_error_from_read_error(e: E) -> std::io::Error { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("Error while reading object data: {}", e), - ) + std::io::Error::other(format!("Error while reading object data: {}", e)) } // ---- diff --git a/src/api/s3/list.rs b/src/api/s3/list.rs index cc1d88a4..58b66388 100644 --- a/src/api/s3/list.rs +++ b/src/api/s3/list.rs @@ -1014,12 +1014,12 @@ mod tests { query.common.prefix = "a/".to_string(); assert_eq!( - common_prefix(objs.get(0).unwrap(), &query.common), + common_prefix(objs.first().unwrap(), &query.common), Some("a/b/") ); query.common.prefix = "a/b/".to_string(); - assert_eq!(common_prefix(objs.get(0).unwrap(), &query.common), None); + assert_eq!(common_prefix(objs.first().unwrap(), &query.common), None); } #[test] diff --git a/src/garage/cli/remote/bucket.rs b/src/garage/cli/remote/bucket.rs index 5085480d..cd0ac87f 100644 --- a/src/garage/cli/remote/bucket.rs +++ b/src/garage/cli/remote/bucket.rs @@ -92,12 +92,7 @@ impl Cli { .await?; // CLI-only checks: the bucket must not have other aliases - if bucket - .global_aliases - .iter() - .find(|a| **a != opt.name) - .is_some() - { + if bucket.global_aliases.iter().any(|a| *a != opt.name) { return Err(Error::Message(format!("Bucket {} still has other global aliases. Use `bucket unalias` to delete them one by one.", opt.name))); } diff --git a/src/net/client.rs b/src/net/client.rs index 20e1dacd..bdb1e6c4 100644 --- a/src/net/client.rs +++ b/src/net/client.rs @@ -180,8 +180,7 @@ impl ClientConn { "Too many inflight requests! RequestID collision. Interrupting previous request." ); let _ = old_ch.send(Box::pin(futures::stream::once(async move { - Err(std::io::Error::new( - std::io::ErrorKind::Other, + Err(std::io::Error::other( "RequestID collision, too many inflight requests", )) }))); diff --git a/src/net/message.rs b/src/net/message.rs index bbd9cede..8d7d612c 100644 --- a/src/net/message.rs +++ b/src/net/message.rs @@ -493,10 +493,7 @@ impl RespEnc { (res_stream, order_tag) } Err(err) => { - let err = std::io::Error::new( - std::io::ErrorKind::Other, - format!("netapp error: {}", err), - ); + let err = std::io::Error::other(format!("netapp error: {}", err)); ( Box::pin(futures::stream::once(async move { Err(err) })), None, diff --git a/src/net/recv.rs b/src/net/recv.rs index 1514b391..50705f24 100644 --- a/src/net/recv.rs +++ b/src/net/recv.rs @@ -62,7 +62,7 @@ pub(crate) trait RecvLoop: Sync + 'static { trace!( "recv_loop({}): in_progress = {:?}", debug_name, - streams.iter().map(|(id, _)| id).collect::>() + streams.keys().collect::>() ); let mut header_id = [0u8; RequestID::BITS as usize / 8]; @@ -79,10 +79,7 @@ pub(crate) trait RecvLoop: Sync + 'static { if size == CANCEL_REQUEST { if let Some(mut stream) = streams.remove(&id) { - stream.send(Err(std::io::Error::new( - std::io::ErrorKind::Other, - "netapp: cancel requested", - ))); + stream.send(Err(std::io::Error::other("netapp: cancel requested"))); stream.end(); } self.cancel_handler(id); diff --git a/src/rpc/layout/helper.rs b/src/rpc/layout/helper.rs index ec4137bc..7ee0721a 100644 --- a/src/rpc/layout/helper.rs +++ b/src/rpc/layout/helper.rs @@ -264,13 +264,13 @@ impl LayoutHelper { .versions .iter() .map(|x| x.version) - .skip_while(|v| { - self.ack_lock + .find(|v| { + !self + .ack_lock .get(v) .map(|x| x.load(Ordering::Relaxed) == 0) .unwrap_or(true) }) - .next() .unwrap_or(self.inner().current().version); let changed = self.update(|layout| { layout diff --git a/src/rpc/layout/manager.rs b/src/rpc/layout/manager.rs index 03e70fa3..3a15cc85 100644 --- a/src/rpc/layout/manager.rs +++ b/src/rpc/layout/manager.rs @@ -116,7 +116,7 @@ impl LayoutManager { pub fn sync_table_until(self: &Arc, table_name: &'static str, version: u64) { let mut table_sync_version = self.table_sync_version.lock().unwrap(); *table_sync_version.get_mut(table_name).unwrap() = version; - let sync_until = table_sync_version.iter().map(|(_, v)| *v).min().unwrap(); + let sync_until = *table_sync_version.values().min().unwrap(); drop(table_sync_version); let mut layout = self.layout.write().unwrap(); diff --git a/src/rpc/system.rs b/src/rpc/system.rs index ee6a3547..107102b0 100644 --- a/src/rpc/system.rs +++ b/src/rpc/system.rs @@ -951,13 +951,13 @@ fn get_rpc_public_addr(config: &Config) -> Option { let filter_subnet: Option = config .rpc_public_addr_subnet .as_ref() - .and_then(|filter_subnet_str| match filter_subnet_str.parse::() { + .map(|filter_subnet_str| match filter_subnet_str.parse::() { Ok(filter_subnet) => { let filter_subnet_trunc = filter_subnet.trunc(); if filter_subnet_trunc != filter_subnet { warn!("`rpc_public_addr_subnet` changed after applying netmask, continuing with {}", filter_subnet.trunc()); } - Some(filter_subnet_trunc) + filter_subnet_trunc } Err(e) => { panic!(