Fix clippy lints related to recent bug

This commit is contained in:
Igor Katson 2024-04-24 14:19:04 +01:00
parent 5dee78227b
commit 3d46d7cb97
No known key found for this signature in database
GPG key ID: B4EC22B66D61A3F5
13 changed files with 89 additions and 78 deletions

View file

@ -190,6 +190,6 @@ impl<'de> serde::de::Deserialize<'de> for ByteBufOwned {
Ok(v.to_owned().into()) Ok(v.to_owned().into())
} }
} }
Ok(deserializer.deserialize_byte_buf(Visitor {})?) deserializer.deserialize_byte_buf(Visitor {})
} }
} }

View file

@ -582,6 +582,7 @@ mod tests {
let full = format!("/tmp/{filename}.bin"); let full = format!("/tmp/{filename}.bin");
let mut f = std::fs::OpenOptions::new() let mut f = std::fs::OpenOptions::new()
.create(true) .create(true)
.truncate(true)
.write(true) .write(true)
.open(full) .open(full)
.unwrap(); .unwrap();

View file

@ -186,13 +186,15 @@ impl ChunkTracker {
&'a self, &'a self,
file_priorities: &'a FilePriorities, file_priorities: &'a FilePriorities,
opened_files: &'a OpenedFiles, opened_files: &'a OpenedFiles,
) -> impl Iterator<Item = usize> + 'a { ) -> impl Iterator<Item = ValidPieceIndex> + 'a {
file_priorities file_priorities
.iter() .iter()
.filter_map(|p| opened_files.get(*p)) .filter_map(|p| opened_files.get(*p))
.filter(|f| !f.approx_is_finished()) .filter(|f| !f.approx_is_finished())
.flat_map(|f| f.iter_piece_priorities()) .flat_map(|f| f.iter_piece_priorities())
.filter(|id| self.queue_pieces[*id]) .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 // None if wrong chunk
@ -264,7 +266,7 @@ impl ChunkTracker {
let chunk_info = self.lengths.chunk_info_from_received_data( let chunk_info = self.lengths.chunk_info_from_received_data(
self.lengths.validate_piece_index(piece.index)?, self.lengths.validate_piece_index(piece.index)?,
piece.begin, 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.lengths.chunk_range(chunk_info.piece_index);
let chunk_range = self.chunk_status.get_mut(chunk_range).unwrap(); 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") anyhow::bail!("bug: shift = 0, this shouldn't have happened")
} }
remaining_file_len -= shift; remaining_file_len -= shift;
current_piece_remaining -= shift as u32; current_piece_remaining -= TryInto::<u32>::try_into(shift)?;
if current_piece_remaining == 0 { if current_piece_remaining == 0 {
let current_piece_have = self.have[current_piece.piece_index.get() as usize]; let current_piece_have = self.have[current_piece.piece_index.get() as usize];
@ -393,13 +395,13 @@ mod tests {
have_pieces.set(0, false); have_pieces.set(0, false);
let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap();
assert_eq!(chunks[0], false); assert!(!chunks[0]);
assert_eq!(chunks[1], false); assert!(!chunks[1]);
assert_eq!(chunks[2], false); assert!(!chunks[2]);
assert_eq!(chunks[3], true); assert!(chunks[3]);
assert_eq!(chunks[4], true); assert!(chunks[4]);
assert_eq!(chunks[5], true); assert!(chunks[5]);
assert_eq!(chunks[6], true); assert!(chunks[6]);
} }
{ {
@ -409,13 +411,13 @@ mod tests {
let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap();
dbg!(&chunks); dbg!(&chunks);
assert_eq!(chunks[0], true); assert!(chunks[0]);
assert_eq!(chunks[1], true); assert!(chunks[1]);
assert_eq!(chunks[2], true); assert!(chunks[2]);
assert_eq!(chunks[3], false); assert!(!chunks[3]);
assert_eq!(chunks[4], false); assert!(!chunks[4]);
assert_eq!(chunks[5], false); assert!(!chunks[5]);
assert_eq!(chunks[6], true); assert!(chunks[6]);
} }
{ {
@ -425,13 +427,13 @@ mod tests {
let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap();
dbg!(&chunks); dbg!(&chunks);
assert_eq!(chunks[0], true); assert!(chunks[0]);
assert_eq!(chunks[1], true); assert!(chunks[1]);
assert_eq!(chunks[2], true); assert!(chunks[2]);
assert_eq!(chunks[3], true); assert!(chunks[3]);
assert_eq!(chunks[4], true); assert!(chunks[4]);
assert_eq!(chunks[5], true); assert!(chunks[5]);
assert_eq!(chunks[6], false); assert!(!chunks[6]);
} }
{ {
@ -451,11 +453,11 @@ mod tests {
let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap();
dbg!(&chunks); dbg!(&chunks);
assert_eq!(chunks[0], true); assert!(chunks[0]);
assert_eq!(chunks[1], true); assert!(chunks[1]);
assert_eq!(chunks[2], false); assert!(!chunks[2]);
assert_eq!(chunks[3], false); assert!(!chunks[3]);
assert_eq!(chunks[4], true); assert!(chunks[4]);
} }
{ {
@ -466,11 +468,11 @@ mod tests {
let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap(); let chunks = compute_chunk_have_status(&l, &have_pieces).unwrap();
dbg!(&chunks); dbg!(&chunks);
assert_eq!(chunks[0], true); assert!(chunks[0]);
assert_eq!(chunks[1], true); assert!(chunks[1]);
assert_eq!(chunks[2], true); assert!(chunks[2]);
assert_eq!(chunks[3], true); assert!(chunks[3]);
assert_eq!(chunks[4], false); assert!(!chunks[4]);
} }
} }
} }
@ -521,9 +523,9 @@ mod tests {
needed_bytes: all_files[0], needed_bytes: all_files[0],
} }
); );
assert_eq!(ct.queue_pieces[0], true); assert!(ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], false); assert!(!ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], false); assert!(!ct.queue_pieces[2]);
// Select only the second file. // Select only the second file.
assert_eq!( assert_eq!(
@ -535,9 +537,9 @@ mod tests {
needed_bytes: piece_len as u64, needed_bytes: piece_len as u64,
} }
); );
assert_eq!(ct.queue_pieces[0], false); assert!(!ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], true); assert!(ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], false); assert!(!ct.queue_pieces[2]);
// Select only the third file (zero sized one!). // Select only the third file (zero sized one!).
assert_eq!( assert_eq!(
@ -549,9 +551,9 @@ mod tests {
needed_bytes: 0, needed_bytes: 0,
} }
); );
assert_eq!(ct.queue_pieces[0], false); assert!(!ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], false); assert!(!ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], false); assert!(!ct.queue_pieces[2]);
// Select only the fourth file. // Select only the fourth file.
assert_eq!( assert_eq!(
@ -563,13 +565,13 @@ mod tests {
needed_bytes: (piece_len + 1) as u64, needed_bytes: (piece_len + 1) as u64,
} }
); );
assert_eq!(ct.queue_pieces[0], false); assert!(!ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], true); assert!(ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], true); assert!(ct.queue_pieces[2]);
// Select first and last file // Select first and last file
assert_eq!( 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(), .unwrap(),
HaveNeededSelected { HaveNeededSelected {
have_bytes: 0, have_bytes: 0,
@ -577,13 +579,13 @@ mod tests {
needed_bytes: all_files[0] + all_files[3] + 1, needed_bytes: all_files[0] + all_files[3] + 1,
} }
); );
assert_eq!(ct.queue_pieces[0], true); assert!(ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], true); assert!(ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], true); assert!(ct.queue_pieces[2]);
// Select all files // Select all files
assert_eq!( 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(), .unwrap(),
HaveNeededSelected { HaveNeededSelected {
have_bytes: 0, have_bytes: 0,
@ -591,8 +593,8 @@ mod tests {
needed_bytes: total_len needed_bytes: total_len
} }
); );
assert_eq!(ct.queue_pieces[0], true); assert!(ct.queue_pieces[0]);
assert_eq!(ct.queue_pieces[1], true); assert!(ct.queue_pieces[1]);
assert_eq!(ct.queue_pieces[2], true); assert!(ct.queue_pieces[2]);
} }
} }

View file

@ -145,7 +145,7 @@ async fn create_torrent_raw<'a>(
length += size as u64; length += size as u64;
piece_checksum.update(&read_buf[..size]); piece_checksum.update(&read_buf[..size]);
remaining_piece_length -= size as u32; remaining_piece_length -= TryInto::<u32>::try_into(size)?;
if remaining_piece_length == 0 { if remaining_piece_length == 0 {
remaining_piece_length = piece_length; remaining_piece_length = piece_length;
piece_hashes.extend_from_slice(&piece_checksum.finish()); piece_hashes.extend_from_slice(&piece_checksum.finish());

View file

@ -145,8 +145,8 @@ impl<'a> FileOps<'a> {
progress.fetch_add(piece_info.len as u64, Ordering::Relaxed); progress.fetch_add(piece_info.len as u64, Ordering::Relaxed);
while piece_remaining > 0 { while piece_remaining > 0 {
let mut to_read_in_file = let mut to_read_in_file: usize =
std::cmp::min(current_file.remaining(), piece_remaining as u64) as 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. // Keep changing the current file to next until we find a file that has greater than 0 length.
while to_read_in_file == 0 { while to_read_in_file == 0 {
@ -157,7 +157,8 @@ impl<'a> FileOps<'a> {
piece_selected |= current_file.full_file_required; piece_selected |= current_file.full_file_required;
to_read_in_file = 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); piece_files.push(current_file.index);
@ -265,7 +266,7 @@ impl<'a> FileOps<'a> {
let file_remaining_len = file_len - absolute_offset; let file_remaining_len = file_len - absolute_offset;
let to_read_in_file = 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(); let mut file_g = self.files[file_idx].file.lock();
trace!( trace!(
"piece={}, handle={}, file_idx={}, seeking to {}. Last received chunk: {:?}", "piece={}, handle={}, file_idx={}, seeking to {}. Last received chunk: {:?}",
@ -332,7 +333,7 @@ impl<'a> FileOps<'a> {
continue; continue;
} }
let file_remaining_len = file_len - absolute_offset; 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(); let mut file_g = self.files[file_idx].file.lock();
trace!( trace!(
@ -385,7 +386,7 @@ impl<'a> FileOps<'a> {
} }
let remaining_len = file_len - absolute_offset; 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(); let mut file_g = self.files[file_idx].file.lock();
trace!( trace!(

View file

@ -23,6 +23,8 @@
//! a facade that works with simple serializable types. //! a facade that works with simple serializable types.
//! //!
#![warn(clippy::cast_possible_truncation)]
pub mod api; pub mod api;
mod api_error; mod api_error;
mod chunk_tracker; mod chunk_tracker;

View file

@ -286,6 +286,7 @@ impl<H: PeerConnectionHandler> PeerConnection<H> {
bail!("disconnecting, to simulate failure in tests"); bail!("disconnecting, to simulate failure in tests");
} }
#[allow(clippy::cast_possible_truncation)]
let sleep_ms = (rand::thread_rng().gen::<f64>() let sleep_ms = (rand::thread_rng().gen::<f64>()
* (tpm.max_random_sleep_ms as f64)) * (tpm.max_random_sleep_ms as f64))
as u64; as u64;

View file

@ -76,18 +76,22 @@ impl HandlerLocked {
anyhow::bail!("metadata size {} is too big", metadata_size); anyhow::bail!("metadata size {} is too big", metadata_size);
} }
let buffer = vec![0u8; metadata_size as usize]; let buffer = vec![0u8; metadata_size as usize];
let total_pieces = (metadata_size as u64).div_ceil(CHUNK_SIZE as u64); let total_pieces: usize = (metadata_size as u64)
let received_pieces = vec![false; total_pieces as usize]; .div_ceil(CHUNK_SIZE as u64)
.try_into()?;
let received_pieces = vec![false; total_pieces];
Ok(Self { Ok(Self {
metadata_size, metadata_size,
received_pieces, received_pieces,
buffer, buffer,
total_pieces: total_pieces as usize, total_pieces,
}) })
} }
fn piece_size(&self, index: u32) -> usize { fn piece_size(&self, index: u32) -> usize {
if index as usize == self.total_pieces - 1 { 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 { } else {
CHUNK_SIZE as usize CHUNK_SIZE as usize
} }
@ -216,7 +220,7 @@ impl PeerConnectionHandler for Handler {
for i in 0..total_pieces { for i in 0..total_pieces {
self.writer_tx self.writer_tx
.send(WriterRequest::Message(Message::Extended( .send(WriterRequest::Message(Message::Extended(
ExtendedMessage::UtMetadata(UtMetadata::Request(i as u32)), ExtendedMessage::UtMetadata(UtMetadata::Request(i.try_into()?)),
)))?; )))?;
} }
Ok(()) Ok(())

View file

@ -54,6 +54,7 @@ impl TorrentStateInitializing {
let file = if self.meta.options.overwrite { let file = if self.meta.options.overwrite {
OpenOptions::new() OpenOptions::new()
.create(true) .create(true)
.truncate(false)
.read(true) .read(true)
.write(true) .write(true)
.open(&full_path) .open(&full_path)

View file

@ -816,6 +816,7 @@ impl<'a> PeerConnectionHandler for &'a PeerHandler {
self.counters self.counters
.outgoing_connections .outgoing_connections
.fetch_add(1, Ordering::Relaxed); .fetch_add(1, Ordering::Relaxed);
#[allow(clippy::cast_possible_truncation)]
self.counters self.counters
.total_time_connecting_ms .total_time_connecting_ms
.fetch_add(connection_time.as_millis() as u64, Ordering::Relaxed); .fetch_add(connection_time.as_millis() as u64, Ordering::Relaxed);
@ -1005,21 +1006,16 @@ impl PeerHandler {
.get_chunks()? .get_chunks()?
.iter_queued_pieces(&g.file_priorities, &self.state.files) .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); n_opt = Some(n);
break; break;
} }
} }
let n_opt = match n_opt { match n_opt {
Some(n_opt) => n_opt, Some(n_opt) => n_opt,
None => return Ok(None), None => return Ok(None),
}; }
self.state
.lengths
.validate_piece_index(n_opt as u32)
.context("bug: invalid piece")?
}; };
g.inflight_pieces.insert( g.inflight_pieces.insert(
n, n,
@ -1309,7 +1305,7 @@ impl PeerHandler {
let chunk_info = match self.state.lengths.chunk_info_from_received_data( let chunk_info = match self.state.lengths.chunk_info_from_received_data(
piece_index, piece_index,
piece.begin, piece.begin,
piece.block.len() as u32, piece.block.len().try_into().context("bug")?,
) { ) {
Some(i) => i, Some(i) => i,
None => { None => {
@ -1453,6 +1449,7 @@ impl PeerHandler {
.stats .stats
.have_bytes .have_bytes
.fetch_add(piece_len, Ordering::Relaxed); .fetch_add(piece_len, Ordering::Relaxed);
#[allow(clippy::cast_possible_truncation)]
self.state.stats.total_piece_download_ms.fetch_add( self.state.stats.total_piece_download_ms.fetch_add(
full_piece_download_time.as_millis() as u64, full_piece_download_time.as_millis() as u64,
Ordering::Relaxed, Ordering::Relaxed,

View file

@ -26,6 +26,7 @@ pub(crate) struct PeerCountersAtomic {
impl PeerCountersAtomic { impl PeerCountersAtomic {
pub(crate) fn on_piece_downloaded(&self, piece_len: u64, elapsed: Duration) { pub(crate) fn on_piece_downloaded(&self, piece_len: u64, elapsed: Duration) {
#[allow(clippy::cast_possible_truncation)]
let elapsed = elapsed.as_millis() as u64; let elapsed = elapsed.as_millis() as u64;
self.total_piece_download_ms self.total_piece_download_ms
.fetch_add(elapsed, Ordering::Release); .fetch_add(elapsed, Ordering::Release);

View file

@ -247,7 +247,7 @@ impl Lengths {
if chunk_index == last_chunk_id { if chunk_index == last_chunk_id {
return Some(last_element_size(piece_length, CHUNK_SIZE)); 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). // 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. // 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.default_chunks_per_piece(), 11);
assert_eq!(l.total_pieces(), 11); assert_eq!(l.total_pieces(), 11);
assert_eq!(l.total_chunks(), 111); assert_eq!(l.total_chunks(), 111);

View file

@ -644,6 +644,7 @@ mod tests {
use std::io::Write; use std::io::Write;
let mut f = std::fs::OpenOptions::new() let mut f = std::fs::OpenOptions::new()
.create(true) .create(true)
.truncate(true)
.write(true) .write(true)
.open("/tmp/test_deserialize_serialize_extended_is_same") .open("/tmp/test_deserialize_serialize_extended_is_same")
.unwrap(); .unwrap();