From a714def8d0b89539e367254bef6d84cd8d97a9ca Mon Sep 17 00:00:00 2001 From: luojia65 Date: Wed, 27 Oct 2021 16:26:57 +0800 Subject: [PATCH 1/2] Add lint `#[must_use]` for ring buffer functions. This lint is in stable Rust now: https://github.com/rust-lang/rust/issues/43302. These changes are noted according to code comment from @whitequark. Some test cases and functions are altered due to changes of #[must_use]. --- src/socket/tcp.rs | 3 ++- src/storage/packet_buffer.rs | 7 +++++-- src/storage/ring_buffer.rs | 38 +++++++++++++++++++++--------------- 3 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index e2d404d..b8fbcdd 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -1875,8 +1875,9 @@ impl<'a> TcpSocket<'a> { payload_len, payload_offset ); - self.rx_buffer + let len_written = self.rx_buffer .write_unallocated(payload_offset, repr.payload); + debug_assert!(len_written == payload_len); } Err(_) => { net_debug!( diff --git a/src/storage/packet_buffer.rs b/src/storage/packet_buffer.rs index 6ce6d17..db318bb 100644 --- a/src/storage/packet_buffer.rs +++ b/src/storage/packet_buffer.rs @@ -102,7 +102,9 @@ impl<'a, H> PacketBuffer<'a, H> { // Add padding to the end of the ring buffer so that the // contiguous window is at the beginning of the ring buffer. *self.metadata_ring.enqueue_one()? = PacketMetadata::padding(contig_window); - self.payload_ring.enqueue_many(contig_window); + // note(discard): function does not write to the result + // enqueued padding buffer location + let _buf_enqueued = self.payload_ring.enqueue_many(contig_window); } } @@ -121,7 +123,8 @@ impl<'a, H> PacketBuffer<'a, H> { let _ = metadata_ring.dequeue_one_with(|metadata| { if metadata.is_padding() { - payload_ring.dequeue_many(metadata.size); + // note(discard): function does not use value of dequeued padding bytes + let _buf_dequeued = payload_ring.dequeue_many(metadata.size); Ok(()) // dequeue metadata } else { Err(Error::Exhausted) // don't dequeue metadata diff --git a/src/storage/ring_buffer.rs b/src/storage/ring_buffer.rs index e802ad1..67dcf6a 100644 --- a/src/storage/ring_buffer.rs +++ b/src/storage/ring_buffer.rs @@ -1,4 +1,5 @@ -// Uncomment the #[must_use]s here once [RFC 1940] hits stable. +// Some of the functions in ring buffer is marked as #[must_use]. It notes that +// these functions may have side effects, and it's implemented by [RFC 1940]. // [RFC 1940]: https://github.com/rust-lang/rust/issues/43302 use core::cmp; @@ -202,7 +203,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// /// This function may return a slice smaller than the given size /// if the free space in the buffer is not contiguous. - // #[must_use] + #[must_use] pub fn enqueue_many(&mut self, size: usize) -> &mut [T] { self.enqueue_many_with(|buf| { let size = cmp::min(size, buf.len()); @@ -213,7 +214,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// Enqueue as many elements from the given slice into the buffer as possible, /// and return the amount of elements that could fit. - // #[must_use] + #[must_use] pub fn enqueue_slice(&mut self, data: &[T]) -> usize where T: Copy, @@ -259,7 +260,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// /// This function may return a slice smaller than the given size /// if the allocated space in the buffer is not contiguous. - // #[must_use] + #[must_use] pub fn dequeue_many(&mut self, size: usize) -> &mut [T] { self.dequeue_many_with(|buf| { let size = cmp::min(size, buf.len()); @@ -270,7 +271,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// Dequeue as many elements from the buffer into the given slice as possible, /// and return the amount of elements that could fit. - // #[must_use] + #[must_use] pub fn dequeue_slice(&mut self, data: &mut [T]) -> usize where T: Copy, @@ -294,7 +295,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { impl<'a, T: 'a> RingBuffer<'a, T> { /// Return the largest contiguous slice of unallocated buffer elements starting /// at the given offset past the last allocated element, and up to the given size. - // #[must_use] + #[must_use] pub fn get_unallocated(&mut self, offset: usize, mut size: usize) -> &mut [T] { let start_at = self.get_idx(self.length + offset); // We can't access past the end of unallocated data. @@ -318,7 +319,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// Write as many elements from the given slice into unallocated buffer elements /// starting at the given offset past the last allocated element, and return /// the amount written. - // #[must_use] + #[must_use] pub fn write_unallocated(&mut self, offset: usize, data: &[T]) -> usize where T: Copy, @@ -349,7 +350,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// Return the largest contiguous slice of allocated buffer elements starting /// at the given offset past the first allocated element, and up to the given size. - // #[must_use] + #[must_use] pub fn get_allocated(&self, offset: usize, mut size: usize) -> &[T] { let start_at = self.get_idx(offset); // We can't read past the end of the allocated data. @@ -373,7 +374,7 @@ impl<'a, T: 'a> RingBuffer<'a, T> { /// Read as many elements from allocated buffer elements into the given slice /// starting at the given offset past the first allocated element, and return /// the amount read. - // #[must_use] + #[must_use] pub fn read_allocated(&mut self, offset: usize, data: &mut [T]) -> usize where T: Copy, @@ -686,7 +687,8 @@ mod test { } assert_eq!(&ring.storage[..], b"abcd........"); - ring.enqueue_many(4); + let buf_enqueued = ring.enqueue_many(4); + assert_eq!(buf_enqueued.len(), 4); assert_eq!(ring.len(), 4); { @@ -730,15 +732,18 @@ mod test { assert_eq!(ring.get_allocated(16, 4), b""); assert_eq!(ring.get_allocated(0, 4), b""); - ring.enqueue_slice(b"abcd"); + let len_enqueued = ring.enqueue_slice(b"abcd"); assert_eq!(ring.get_allocated(0, 8), b"abcd"); + assert_eq!(len_enqueued, 4); - ring.enqueue_slice(b"efghijkl"); + let len_enqueued = ring.enqueue_slice(b"efghijkl"); ring.dequeue_many(4).copy_from_slice(b"...."); assert_eq!(ring.get_allocated(4, 8), b"ijkl"); + assert_eq!(len_enqueued, 8); - ring.enqueue_slice(b"abcd"); + let len_enqueued = ring.enqueue_slice(b"abcd"); assert_eq!(ring.get_allocated(4, 8), b"ijkl"); + assert_eq!(len_enqueued, 4); } #[test] @@ -782,10 +787,11 @@ mod test { #[test] fn test_buffer_write_wholly() { let mut ring = RingBuffer::new(vec![b'.'; 8]); - ring.enqueue_many(2).copy_from_slice(b"xx"); - ring.enqueue_many(2).copy_from_slice(b"xx"); + ring.enqueue_many(2).copy_from_slice(b"ab"); + ring.enqueue_many(2).copy_from_slice(b"cd"); assert_eq!(ring.len(), 4); - ring.dequeue_many(4); + let buf_dequeued = ring.dequeue_many(4); + assert_eq!(buf_dequeued, b"abcd"); assert_eq!(ring.len(), 0); let large = ring.enqueue_many(8); From c111bee3b6156f087588f6eb25a7ce9d91bfed90 Mon Sep 17 00:00:00 2001 From: luojia65 Date: Wed, 27 Oct 2021 16:35:05 +0800 Subject: [PATCH 2/2] Code format using `cargo fmt` --- src/socket/tcp.rs | 3 ++- src/storage/packet_buffer.rs | 2 +- src/storage/ring_buffer.rs | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/socket/tcp.rs b/src/socket/tcp.rs index b8fbcdd..064fd5f 100644 --- a/src/socket/tcp.rs +++ b/src/socket/tcp.rs @@ -1875,7 +1875,8 @@ impl<'a> TcpSocket<'a> { payload_len, payload_offset ); - let len_written = self.rx_buffer + let len_written = self + .rx_buffer .write_unallocated(payload_offset, repr.payload); debug_assert!(len_written == payload_len); } diff --git a/src/storage/packet_buffer.rs b/src/storage/packet_buffer.rs index db318bb..4a5080b 100644 --- a/src/storage/packet_buffer.rs +++ b/src/storage/packet_buffer.rs @@ -102,7 +102,7 @@ impl<'a, H> PacketBuffer<'a, H> { // Add padding to the end of the ring buffer so that the // contiguous window is at the beginning of the ring buffer. *self.metadata_ring.enqueue_one()? = PacketMetadata::padding(contig_window); - // note(discard): function does not write to the result + // note(discard): function does not write to the result // enqueued padding buffer location let _buf_enqueued = self.payload_ring.enqueue_many(contig_window); } diff --git a/src/storage/ring_buffer.rs b/src/storage/ring_buffer.rs index 67dcf6a..f54837b 100644 --- a/src/storage/ring_buffer.rs +++ b/src/storage/ring_buffer.rs @@ -1,4 +1,4 @@ -// Some of the functions in ring buffer is marked as #[must_use]. It notes that +// Some of the functions in ring buffer is marked as #[must_use]. It notes that // these functions may have side effects, and it's implemented by [RFC 1940]. // [RFC 1940]: https://github.com/rust-lang/rust/issues/43302