Skip to content

Commit

Permalink
Merge #561
Browse files Browse the repository at this point in the history
561: Add lint `#[must_use]` for ring buffer functions r=Dirbaio a=luojia65

This pull request adds `#[must_use]` flag on several ring buffer functions that have side-effects.

This lint is in stable Rust now: rust-lang/rust#43302, these changes are noted according to code comment from `@whitequark:` f65bc8a.

Some test cases and functions are altered due to changes of `#[must_use]`.

Co-authored-by: luojia65 <me@luojia.cc>
  • Loading branch information
bors[bot] and luojia65 committed Nov 3, 2021
2 parents d5af245 + 7a0df7d commit 24fa299
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 19 deletions.
4 changes: 3 additions & 1 deletion src/socket/tcp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1875,8 +1875,10 @@ 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!(
Expand Down
7 changes: 5 additions & 2 deletions src/storage/packet_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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
Expand Down
38 changes: 22 additions & 16 deletions src/storage/ring_buffer.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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());
Expand All @@ -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,
Expand Down Expand Up @@ -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());
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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);

{
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 24fa299

Please sign in to comment.