From 3d46d7cb97f9c31458950b6fca88239ff1dcb2e6 Mon Sep 17 00:00:00 2001 From: Igor Katson Date: Wed, 24 Apr 2024 14:19:04 +0100 Subject: [PATCH] Fix clippy lints related to recent bug --- crates/buffers/src/lib.rs | 2 +- crates/dht/src/bprotocol.rs | 1 + crates/librqbit/src/chunk_tracker.rs | 110 +++++++++--------- crates/librqbit/src/create_torrent_file.rs | 2 +- crates/librqbit/src/file_ops.rs | 13 ++- crates/librqbit/src/lib.rs | 2 + crates/librqbit/src/peer_connection.rs | 1 + crates/librqbit/src/peer_info_reader/mod.rs | 14 ++- .../src/torrent_state/initializing.rs | 1 + crates/librqbit/src/torrent_state/live/mod.rs | 15 +-- .../torrent_state/live/peer/stats/atomic.rs | 1 + crates/librqbit_core/src/lengths.rs | 4 +- crates/peer_binary_protocol/src/lib.rs | 1 + 13 files changed, 89 insertions(+), 78 deletions(-) diff --git a/crates/buffers/src/lib.rs b/crates/buffers/src/lib.rs index 934db37..54f750c 100644 --- a/crates/buffers/src/lib.rs +++ b/crates/buffers/src/lib.rs @@ -190,6 +190,6 @@ impl<'de> serde::de::Deserialize<'de> for ByteBufOwned { Ok(v.to_owned().into()) } } - Ok(deserializer.deserialize_byte_buf(Visitor {})?) + deserializer.deserialize_byte_buf(Visitor {}) } } diff --git a/crates/dht/src/bprotocol.rs b/crates/dht/src/bprotocol.rs index 2c5335b..49ed116 100644 --- a/crates/dht/src/bprotocol.rs +++ b/crates/dht/src/bprotocol.rs @@ -582,6 +582,7 @@ mod tests { let full = format!("/tmp/{filename}.bin"); let mut f = std::fs::OpenOptions::new() .create(true) + .truncate(true) .write(true) .open(full) .unwrap(); diff --git a/crates/librqbit/src/chunk_tracker.rs b/crates/librqbit/src/chunk_tracker.rs index fc4a6c4..b830186 100644 --- a/crates/librqbit/src/chunk_tracker.rs +++ b/crates/librqbit/src/chunk_tracker.rs @@ -186,13 +186,15 @@ impl ChunkTracker { &'a self, file_priorities: &'a FilePriorities, opened_files: &'a OpenedFiles, - ) -> impl Iterator + 'a { + ) -> impl Iterator + 'a { file_priorities .iter() .filter_map(|p| opened_files.get(*p)) .filter(|f| !f.approx_is_finished()) .flat_map(|f| f.iter_piece_priorities()) .filter(|id| self.queue_pieces[*id]) + .filter_map(|id| id.try_into().ok()) + .filter_map(|id| self.lengths.validate_piece_index(id)) } // None if wrong chunk @@ -264,7 +266,7 @@ impl ChunkTracker { let chunk_info = self.lengths.chunk_info_from_received_data( self.lengths.validate_piece_index(piece.index)?, piece.begin, - piece.block.as_ref().len() as u32, + piece.block.as_ref().len().try_into().unwrap(), )?; let chunk_range = self.lengths.chunk_range(chunk_info.piece_index); let chunk_range = self.chunk_status.get_mut(chunk_range).unwrap(); @@ -316,7 +318,7 @@ impl ChunkTracker { anyhow::bail!("bug: shift = 0, this shouldn't have happened") } remaining_file_len -= shift; - current_piece_remaining -= shift as u32; + current_piece_remaining -= TryInto::::try_into(shift)?; if current_piece_remaining == 0 { let current_piece_have = self.have[current_piece.piece_index.get() as usize]; @@ -393,13 +395,13 @@ mod tests { have_pieces.set(0, false); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); - assert_eq!(chunks[0], false); - assert_eq!(chunks[1], false); - assert_eq!(chunks[2], false); - assert_eq!(chunks[3], true); - assert_eq!(chunks[4], true); - assert_eq!(chunks[5], true); - assert_eq!(chunks[6], true); + assert!(!chunks[0]); + assert!(!chunks[1]); + assert!(!chunks[2]); + assert!(chunks[3]); + assert!(chunks[4]); + assert!(chunks[5]); + assert!(chunks[6]); } { @@ -409,13 +411,13 @@ mod tests { let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); dbg!(&chunks); - assert_eq!(chunks[0], true); - assert_eq!(chunks[1], true); - assert_eq!(chunks[2], true); - assert_eq!(chunks[3], false); - assert_eq!(chunks[4], false); - assert_eq!(chunks[5], false); - assert_eq!(chunks[6], true); + assert!(chunks[0]); + assert!(chunks[1]); + assert!(chunks[2]); + assert!(!chunks[3]); + assert!(!chunks[4]); + assert!(!chunks[5]); + assert!(chunks[6]); } { @@ -425,13 +427,13 @@ mod tests { let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); dbg!(&chunks); - assert_eq!(chunks[0], true); - assert_eq!(chunks[1], true); - assert_eq!(chunks[2], true); - assert_eq!(chunks[3], true); - assert_eq!(chunks[4], true); - assert_eq!(chunks[5], true); - assert_eq!(chunks[6], false); + assert!(chunks[0]); + assert!(chunks[1]); + assert!(chunks[2]); + assert!(chunks[3]); + assert!(chunks[4]); + assert!(chunks[5]); + assert!(!chunks[6]); } { @@ -451,11 +453,11 @@ mod tests { let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); dbg!(&chunks); - assert_eq!(chunks[0], true); - assert_eq!(chunks[1], true); - assert_eq!(chunks[2], false); - assert_eq!(chunks[3], false); - assert_eq!(chunks[4], true); + assert!(chunks[0]); + assert!(chunks[1]); + assert!(!chunks[2]); + assert!(!chunks[3]); + assert!(chunks[4]); } { @@ -466,11 +468,11 @@ mod tests { let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); dbg!(&chunks); - assert_eq!(chunks[0], true); - assert_eq!(chunks[1], true); - assert_eq!(chunks[2], true); - assert_eq!(chunks[3], true); - assert_eq!(chunks[4], false); + assert!(chunks[0]); + assert!(chunks[1]); + assert!(chunks[2]); + assert!(chunks[3]); + assert!(!chunks[4]); } } } @@ -521,9 +523,9 @@ mod tests { needed_bytes: all_files[0], } ); - assert_eq!(ct.queue_pieces[0], true); - assert_eq!(ct.queue_pieces[1], false); - assert_eq!(ct.queue_pieces[2], false); + assert!(ct.queue_pieces[0]); + assert!(!ct.queue_pieces[1]); + assert!(!ct.queue_pieces[2]); // Select only the second file. assert_eq!( @@ -535,9 +537,9 @@ mod tests { needed_bytes: piece_len as u64, } ); - assert_eq!(ct.queue_pieces[0], false); - assert_eq!(ct.queue_pieces[1], true); - assert_eq!(ct.queue_pieces[2], false); + assert!(!ct.queue_pieces[0]); + assert!(ct.queue_pieces[1]); + assert!(!ct.queue_pieces[2]); // Select only the third file (zero sized one!). assert_eq!( @@ -549,9 +551,9 @@ mod tests { needed_bytes: 0, } ); - assert_eq!(ct.queue_pieces[0], false); - assert_eq!(ct.queue_pieces[1], false); - assert_eq!(ct.queue_pieces[2], false); + assert!(!ct.queue_pieces[0]); + assert!(!ct.queue_pieces[1]); + assert!(!ct.queue_pieces[2]); // Select only the fourth file. assert_eq!( @@ -563,13 +565,13 @@ mod tests { needed_bytes: (piece_len + 1) as u64, } ); - assert_eq!(ct.queue_pieces[0], false); - assert_eq!(ct.queue_pieces[1], true); - assert_eq!(ct.queue_pieces[2], true); + assert!(!ct.queue_pieces[0]); + assert!(ct.queue_pieces[1]); + assert!(ct.queue_pieces[2]); // Select first and last file assert_eq!( - ct.update_only_files(all_files.clone(), &HashSet::from_iter([0, 3])) + ct.update_only_files(all_files, &HashSet::from_iter([0, 3])) .unwrap(), HaveNeededSelected { have_bytes: 0, @@ -577,13 +579,13 @@ mod tests { needed_bytes: all_files[0] + all_files[3] + 1, } ); - assert_eq!(ct.queue_pieces[0], true); - assert_eq!(ct.queue_pieces[1], true); - assert_eq!(ct.queue_pieces[2], true); + assert!(ct.queue_pieces[0]); + assert!(ct.queue_pieces[1]); + assert!(ct.queue_pieces[2]); // Select all files assert_eq!( - ct.update_only_files(all_files.clone(), &HashSet::from_iter([0, 1, 2, 3])) + ct.update_only_files(all_files, &HashSet::from_iter([0, 1, 2, 3])) .unwrap(), HaveNeededSelected { have_bytes: 0, @@ -591,8 +593,8 @@ mod tests { needed_bytes: total_len } ); - assert_eq!(ct.queue_pieces[0], true); - assert_eq!(ct.queue_pieces[1], true); - assert_eq!(ct.queue_pieces[2], true); + assert!(ct.queue_pieces[0]); + assert!(ct.queue_pieces[1]); + assert!(ct.queue_pieces[2]); } } diff --git a/crates/librqbit/src/create_torrent_file.rs b/crates/librqbit/src/create_torrent_file.rs index 1d8f179..fe78ca0 100644 --- a/crates/librqbit/src/create_torrent_file.rs +++ b/crates/librqbit/src/create_torrent_file.rs @@ -145,7 +145,7 @@ async fn create_torrent_raw<'a>( length += size as u64; piece_checksum.update(&read_buf[..size]); - remaining_piece_length -= size as u32; + remaining_piece_length -= TryInto::::try_into(size)?; if remaining_piece_length == 0 { remaining_piece_length = piece_length; piece_hashes.extend_from_slice(&piece_checksum.finish()); diff --git a/crates/librqbit/src/file_ops.rs b/crates/librqbit/src/file_ops.rs index b0f8daf..c186de3 100644 --- a/crates/librqbit/src/file_ops.rs +++ b/crates/librqbit/src/file_ops.rs @@ -145,8 +145,8 @@ impl<'a> FileOps<'a> { progress.fetch_add(piece_info.len as u64, Ordering::Relaxed); while piece_remaining > 0 { - let mut to_read_in_file = - std::cmp::min(current_file.remaining(), piece_remaining as u64) as usize; + let mut to_read_in_file: usize = + std::cmp::min(current_file.remaining(), piece_remaining as u64).try_into()?; // Keep changing the current file to next until we find a file that has greater than 0 length. while to_read_in_file == 0 { @@ -157,7 +157,8 @@ impl<'a> FileOps<'a> { piece_selected |= current_file.full_file_required; to_read_in_file = - std::cmp::min(current_file.remaining(), piece_remaining as u64) as usize; + std::cmp::min(current_file.remaining(), piece_remaining as u64) + .try_into()?; } piece_files.push(current_file.index); @@ -265,7 +266,7 @@ impl<'a> FileOps<'a> { let file_remaining_len = file_len - absolute_offset; let to_read_in_file = - std::cmp::min(file_remaining_len, piece_remaining_bytes as u64) as usize; + std::cmp::min(file_remaining_len, piece_remaining_bytes as u64).try_into()?; let mut file_g = self.files[file_idx].file.lock(); trace!( "piece={}, handle={}, file_idx={}, seeking to {}. Last received chunk: {:?}", @@ -332,7 +333,7 @@ impl<'a> FileOps<'a> { continue; } let file_remaining_len = file_len - absolute_offset; - let to_read_in_file = std::cmp::min(file_remaining_len, buf.len() as u64) as usize; + let to_read_in_file = std::cmp::min(file_remaining_len, buf.len() as u64).try_into()?; let mut file_g = self.files[file_idx].file.lock(); trace!( @@ -385,7 +386,7 @@ impl<'a> FileOps<'a> { } let remaining_len = file_len - absolute_offset; - let to_write = std::cmp::min(buf.len() as u64, remaining_len) as usize; + let to_write = std::cmp::min(buf.len() as u64, remaining_len).try_into()?; let mut file_g = self.files[file_idx].file.lock(); trace!( diff --git a/crates/librqbit/src/lib.rs b/crates/librqbit/src/lib.rs index 47bbb7a..6a423f2 100644 --- a/crates/librqbit/src/lib.rs +++ b/crates/librqbit/src/lib.rs @@ -23,6 +23,8 @@ //! a facade that works with simple serializable types. //! +#![warn(clippy::cast_possible_truncation)] + pub mod api; mod api_error; mod chunk_tracker; diff --git a/crates/librqbit/src/peer_connection.rs b/crates/librqbit/src/peer_connection.rs index e5d7096..3468cd7 100644 --- a/crates/librqbit/src/peer_connection.rs +++ b/crates/librqbit/src/peer_connection.rs @@ -286,6 +286,7 @@ impl PeerConnection { bail!("disconnecting, to simulate failure in tests"); } + #[allow(clippy::cast_possible_truncation)] let sleep_ms = (rand::thread_rng().gen::() * (tpm.max_random_sleep_ms as f64)) as u64; diff --git a/crates/librqbit/src/peer_info_reader/mod.rs b/crates/librqbit/src/peer_info_reader/mod.rs index bd31e27..2f11a5e 100644 --- a/crates/librqbit/src/peer_info_reader/mod.rs +++ b/crates/librqbit/src/peer_info_reader/mod.rs @@ -76,18 +76,22 @@ impl HandlerLocked { anyhow::bail!("metadata size {} is too big", metadata_size); } let buffer = vec![0u8; metadata_size as usize]; - let total_pieces = (metadata_size as u64).div_ceil(CHUNK_SIZE as u64); - let received_pieces = vec![false; total_pieces as usize]; + let total_pieces: usize = (metadata_size as u64) + .div_ceil(CHUNK_SIZE as u64) + .try_into()?; + let received_pieces = vec![false; total_pieces]; Ok(Self { metadata_size, received_pieces, buffer, - total_pieces: total_pieces as usize, + total_pieces, }) } fn piece_size(&self, index: u32) -> usize { if index as usize == self.total_pieces - 1 { - last_element_size(self.metadata_size as u64, CHUNK_SIZE as u64) as usize + last_element_size(self.metadata_size as u64, CHUNK_SIZE as u64) + .try_into() + .unwrap() } else { CHUNK_SIZE as usize } @@ -216,7 +220,7 @@ impl PeerConnectionHandler for Handler { for i in 0..total_pieces { self.writer_tx .send(WriterRequest::Message(Message::Extended( - ExtendedMessage::UtMetadata(UtMetadata::Request(i as u32)), + ExtendedMessage::UtMetadata(UtMetadata::Request(i.try_into()?)), )))?; } Ok(()) diff --git a/crates/librqbit/src/torrent_state/initializing.rs b/crates/librqbit/src/torrent_state/initializing.rs index c405e56..d77634e 100644 --- a/crates/librqbit/src/torrent_state/initializing.rs +++ b/crates/librqbit/src/torrent_state/initializing.rs @@ -54,6 +54,7 @@ impl TorrentStateInitializing { let file = if self.meta.options.overwrite { OpenOptions::new() .create(true) + .truncate(false) .read(true) .write(true) .open(&full_path) diff --git a/crates/librqbit/src/torrent_state/live/mod.rs b/crates/librqbit/src/torrent_state/live/mod.rs index 56e90ee..38e5399 100644 --- a/crates/librqbit/src/torrent_state/live/mod.rs +++ b/crates/librqbit/src/torrent_state/live/mod.rs @@ -816,6 +816,7 @@ impl<'a> PeerConnectionHandler for &'a PeerHandler { self.counters .outgoing_connections .fetch_add(1, Ordering::Relaxed); + #[allow(clippy::cast_possible_truncation)] self.counters .total_time_connecting_ms .fetch_add(connection_time.as_millis() as u64, Ordering::Relaxed); @@ -1005,21 +1006,16 @@ impl PeerHandler { .get_chunks()? .iter_queued_pieces(&g.file_priorities, &self.state.files) { - if bf.get(n).map(|v| *v) == Some(true) { + if bf.get(n.get() as usize).map(|v| *v) == Some(true) { n_opt = Some(n); break; } } - let n_opt = match n_opt { + match n_opt { Some(n_opt) => n_opt, None => return Ok(None), - }; - - self.state - .lengths - .validate_piece_index(n_opt as u32) - .context("bug: invalid piece")? + } }; g.inflight_pieces.insert( n, @@ -1309,7 +1305,7 @@ impl PeerHandler { let chunk_info = match self.state.lengths.chunk_info_from_received_data( piece_index, piece.begin, - piece.block.len() as u32, + piece.block.len().try_into().context("bug")?, ) { Some(i) => i, None => { @@ -1453,6 +1449,7 @@ impl PeerHandler { .stats .have_bytes .fetch_add(piece_len, Ordering::Relaxed); + #[allow(clippy::cast_possible_truncation)] self.state.stats.total_piece_download_ms.fetch_add( full_piece_download_time.as_millis() as u64, Ordering::Relaxed, diff --git a/crates/librqbit/src/torrent_state/live/peer/stats/atomic.rs b/crates/librqbit/src/torrent_state/live/peer/stats/atomic.rs index ce9ed68..6b284b4 100644 --- a/crates/librqbit/src/torrent_state/live/peer/stats/atomic.rs +++ b/crates/librqbit/src/torrent_state/live/peer/stats/atomic.rs @@ -26,6 +26,7 @@ pub(crate) struct PeerCountersAtomic { impl PeerCountersAtomic { pub(crate) fn on_piece_downloaded(&self, piece_len: u64, elapsed: Duration) { + #[allow(clippy::cast_possible_truncation)] let elapsed = elapsed.as_millis() as u64; self.total_piece_download_ms .fetch_add(elapsed, Ordering::Release); diff --git a/crates/librqbit_core/src/lengths.rs b/crates/librqbit_core/src/lengths.rs index 3e76463..96e2d73 100644 --- a/crates/librqbit_core/src/lengths.rs +++ b/crates/librqbit_core/src/lengths.rs @@ -247,7 +247,7 @@ impl Lengths { if chunk_index == last_chunk_id { return Some(last_element_size(piece_length, CHUNK_SIZE)); } - return None; + None } // How many bytes out of the given piece are present in the given file (by offset and len). @@ -558,7 +558,7 @@ mod tests { // A few more examples with longer values and weird inputs. - let l = Lengths::new(16384_1_1, 16384_1).unwrap(); + let l = Lengths::new(1_638_411, 163_841).unwrap(); assert_eq!(l.default_chunks_per_piece(), 11); assert_eq!(l.total_pieces(), 11); assert_eq!(l.total_chunks(), 111); diff --git a/crates/peer_binary_protocol/src/lib.rs b/crates/peer_binary_protocol/src/lib.rs index 21cd4be..d3e5d50 100644 --- a/crates/peer_binary_protocol/src/lib.rs +++ b/crates/peer_binary_protocol/src/lib.rs @@ -644,6 +644,7 @@ mod tests { use std::io::Write; let mut f = std::fs::OpenOptions::new() .create(true) + .truncate(true) .write(true) .open("/tmp/test_deserialize_serialize_extended_is_same") .unwrap();