andreacorbellini / rust-circular-buffer Goto Github PK

The use of indices in the documentation (e.g., get) shows under-filled buffers, where the index in the view and the underlying buffer match. A couple of examples of how the index behaves in case of an overfilled buffer would be useful.

For example, given

use circular_buffer::CircularBuffer;

let mut buf = CircularBuffer::<3, char>::new();
buf.push_back('a');
buf.push_back('b');
buf.push_back('c');
buf.push_back('d');

Which of the following is correct?

assert_eq!(buf.get(0), Some(&'d'));

OR

assert_eq!(buf.get(0), Some(&'b'));

In the first case, the index is static relative to the underlying memory, which can be convenient. In the latter, the index of an element changes every time something is appended to the buffer. A corrolary of the former would be that it would be nice to have access to the index of the front and back. I would be happy to make a PR for some getters if this turns out to be the case.

The usecase I have in mind is that I want to buffer some data and grab some items that I've flagged in the past (that are still definitely within the buffer).

I ran into this while setting up a unit test that I expected to panic. It's fairly simple to reproduce:

let buffer: CircularBuffer<2; u8> = [0x01, 0x02, 0x03].into();

The issue seems to be here https://github.com/andreacorbellini/rust-circular-buffer/blob/master/src/lib.rs#L1686. I guess this is a design choice, but I don't think I'd ever want to silently discard array elements while constructing a circular buffer.

It's a small (breaking) change, and I'm happy to open the MR for it. Thanks for the awesome library!

The BSD-3-Clause license states that the copyright notice, list of conditions and disclaimer must be reproduced in all source and binary distributions. Since no license text appears in this repository or in the released crate, it’s not possible to comply with the license stated in Cargo.toml:

Please add the necessary license text, preferably in a file called LICENSE.

This should be important for all potential users; it’s important for me so I can add a rust-circular-buffer package to Fedora Linux without guessing at the correct license text.

Function add_mod does not seem to live up to its documentation regarding "without risk of overflows if ...". Perhaps this case is not relevant, but then this comment should be less unequivocal.

/// Returns `(x + y) % m` without risk of overflows if `x + y` cannot fit in `usize`.
const fn add_mod(x: usize, y: usize, m: usize) -> usize {
    let (z, overflow) = x.overflowing_add(y);
    z % m + (overflow as usize) * (usize::MAX % m + 1)
}

#[test]
fn t() {
    assert_eq!(0, add_mod(usize::MAX, usize::MAX, usize::MAX));
}

thread 't' panicked at 'assertion failed: (left == right)
left: 0,
right: 18446744073709551615', src/lib.rs:8:5

This line can (always?) panic:

thread 'tokio-runtime-worker' panicked at 'range start index 7 out of range for slice of length 0', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/circular-buffer-0.1.5/src/lib.rs:1780:46

Would you accept a PR that adds a variant of CircularBuffer that has a runtime fixed capacity instead of compile-time fixed capacity?

That variant would use a boxed slice (Box<MaybeUninit<T>>) as backing storage instead of an array und thus would only be available under the use_std feature flag. I think almost all method implementations could be shared between the variants using a macro.

Hallo,

it would be very handy if the push_front() and it's corresponding push_back() method return the dropped element.

I have some logic which needs to clean up other stuff in the case an element gets dropped from a CircularBuffer:

let mut revisions = CircularBuffer::new()

...

if let Err(revision) = revisions.try_push_front(revision) {
    if let Some(oldest_revision) = revisions.pop_back() {
        self.store.remove(oldest_revision.hash());
    }
    revisions.try_push_front(revision)
        .expect("After removing the oldest revision, the buffer should now have space to push a new revision.")
}

A fn push_front(&mut self, item: T) -> Option<T> method would reduce the code above to:

if let Some(oldest_revision) = revisions.push_front(revision) {
    self.store.remove(oldest_revision.hash());
}

Taking a few glimpses at the source code I found a few spots that made a little confused.

Here for example:

Why does it say that truncate_front(0) is equivalent to clear()? I assume it is supposed to clear the buffer, but it only does an early return, without changing any cursors or dropping any values.

It also says "The start of the buffer is shrunk before dropping, so no value will be dropped twice in case of panics."
When in reality the start is changed after calling self.drop_range.

Are you planning on actively maintaining this crate or is it just a small side-project?