4
\$\begingroup\$

I implemented this generic ringbuffer over a const generic array with the usage of the MaybeUninit type. Is there something which is maybe unsound (having the lines with drop_in_place in mind) or which could be optimized? Thanks in advance

#![feature(inline_const)]

use core::mem::MaybeUninit;
use core::ptr;

struct Ringbuffer<T, const N: usize> { 
    buffer: [MaybeUninit<T>; N],
    read: usize,
    write: usize,
    size: usize
}

impl <T, const N: usize> Ringbuffer<T, N> {
    pub const fn new() -> Self {
        Self {
            buffer: [const { MaybeUninit::<T>::zeroed() }; N],
            read: 0,
            write: 0,
            size: 0,
        }
    }

    pub fn push(&mut self, value: T) {
        if N == 0 {
            return;
        }
    
        if self.size >= N {
            unsafe {
               ptr::drop_in_place(self.buffer[self.write].as_mut_ptr()); 
            }
        
            self.buffer[self.write].write(value);
            self.wrap_inc_write();
        
        } else {
            self.buffer[self.write].write(value);
            self.wrap_inc_write();
            self.inc_size();
        }
    }

    pub fn pop(&mut self)  -> Option<T> {
        if N == 0 {
            return None;
        }
    
        if self.is_empty() {
            return None;
        }
    
        let ret = unsafe {
            self.buffer[self.read].assume_init_read()
        };
    
        self.wrap_inc_read();
        self.dec_size();
    
        Some(ret)
    }

    pub fn skip(&mut self) {
    
        if N == 0 {
            return;
        }
    
        if self.is_empty() {
            return;
        }
    
        unsafe {
            ptr::drop_in_place(self.buffer[self.read].as_mut_ptr());
        }
    
        self.wrap_inc_read();
        self.dec_size();
    }

    pub fn peek(&mut self) -> Option<&T> {
        if N == 0 {
            return None;
        }
    
        if self.is_empty() {
            return None;
        }
    
        let ret = unsafe {
            self.buffer[self.read].assume_init_ref()  
        };
    
        Some(ret)
    }

    pub fn size(&self) -> usize {
        self.size
    }

    pub fn capacity(&self) -> usize {
        N
    }

    pub fn is_empty(&self) -> bool {
        self.size == 0
    }

    pub fn is_full(&self) -> bool {
        self.size == N
    }

    fn wrap_inc_read(&mut self) {
        self.read = (self.read + 1) % N;
    }

    fn wrap_inc_write(&mut self) {
        self.write = (self.write + 1) % N;
    }

    fn inc_size(&mut self) {
        self.size += 1;
    }

    fn dec_size(&mut self) {
        self.size -= 1;
    }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Do you know heapless::HistoryBuffer? \$\endgroup\$ Nov 29, 2023 at 16:52
  • \$\begingroup\$ No, I didn't know that one. Thanks, but I was mostly implementing this because I want to keep dependencies low and for practice. But thanks! \$\endgroup\$
    – Lockna
    Nov 30, 2023 at 10:26

1 Answer 1

2
\$\begingroup\$

Your code looks perfectly sound to me after looking through docs and the code. Nothing to add, but there is one more way for implementing your push, one that may be effectively the same or slower, though still using some unsafe:

        if self.size >= N {
            unsafe {
                mem::replace(&mut self.buffer[self.write], MaybeUninit::new(value)).assume_init();
            }
            self.wrap_inc_write();
        }

All you need to know is how MaybeUninit works, and this: https://doc.rust-lang.org/core/ptr/fn.drop_in_place.html#safety

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thanks for your comment! On thing I forgot is to drop the elements when the Ringbuffer gets dropped, else their destructor never gets run. play.rust-lang.org/… I verified it with this little example, when I do not implement Drop for Ringbuffer like this, only the value 1 is dropped. \$\endgroup\$
    – Lockna
    Dec 18, 2023 at 9:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.