On 29-09-22, 15:55, Miguel Ojeda wrote:
It looks like a container whose elements get invalidated, so `read_edge_event` could require an exclusive reference to `buffer` in Rust, that way you cannot keep borrows to its elements like `ev` if you want to call it. But of course this requires tying the lifetime of the events to that of the buffer.
What about the below code changes on top of V6 ?
Changes: - Removed BufferInternal. - Event contains a reference to the Buffer now, with lifetime. - read_edge_event() expects a mutable reference to buffer, to make it exclusive, i.e. disallow any previous Event references to exist at compilation itself.
diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index ce583916a2e3..f5426459d779 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -3,12 +3,11 @@ // Copyright 2022 Linaro Ltd. All Rights Reserved. // Viresh Kumar viresh.kumar@linaro.org
-use std::sync::Arc; use std::time::Duration;
use vmm_sys_util::errno::Error as Errno;
-use super::{edge::event::BufferInternal, gpiod, EdgeKind, Error, Offset, OperationType, Result}; +use super::{edge::event::Buffer, gpiod, EdgeKind, Error, Offset, OperationType, Result};
/// Line edge events handling /// @@ -22,15 +21,15 @@ use super::{edge::event::BufferInternal, gpiod, EdgeKind, Error, Offset, Operati /// number of events are being read.
#[derive(Debug, Eq, PartialEq)] -pub struct Event { - ibuffer: Option<Arc<BufferInternal>>, +pub struct Event<'b> { + buffer: Option<&'b Buffer>, event: *mut gpiod::gpiod_edge_event, }
-impl Event { +impl<'b> Event<'b> { /// Get an event stored in the buffer. - pub(crate) fn new(ibuffer: &Arc<BufferInternal>, index: u64) -> Result<Self> { - let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(ibuffer.buffer(), index) }; + pub(crate) fn new(buffer: &'b Buffer, index: u64) -> Result<Self> { + let event = unsafe { gpiod::gpiod_edge_event_buffer_get_event(buffer.buffer(), index) }; if event.is_null() { return Err(Error::OperationFailed( OperationType::EdgeEventBufferGetEvent, @@ -39,7 +38,7 @@ impl Event { }
Ok(Self { - ibuffer: Some(ibuffer.clone()), + buffer: Some(buffer), event, }) } @@ -54,7 +53,7 @@ impl Event { }
Ok(Self { - ibuffer: None, + buffer: None, event, }) } @@ -91,11 +90,11 @@ impl Event { } }
-impl Drop for Event { +impl<'b> Drop for Event<'b> { /// Free the edge event. fn drop(&mut self) { // Free the event only if a copy is made - if self.ibuffer.is_none() { + if self.buffer.is_none() { unsafe { gpiod::gpiod_edge_event_free(self.event) }; } } diff --git a/bindings/rust/libgpiod/src/event_buffer.rs b/bindings/rust/libgpiod/src/event_buffer.rs index e272e7aa9e9d..11c8b5e1d7c9 100644 --- a/bindings/rust/libgpiod/src/event_buffer.rs +++ b/bindings/rust/libgpiod/src/event_buffer.rs @@ -4,7 +4,6 @@ // Viresh Kumar viresh.kumar@linaro.org
use std::os::raw::c_ulong; -use std::sync::Arc;
use vmm_sys_util::errno::Error as Errno;
@@ -12,11 +11,11 @@ use super::{edge, gpiod, Error, OperationType, Result};
/// Line edge events buffer #[derive(Clone, Debug, Eq, PartialEq)] -pub(crate) struct BufferInternal { +pub struct Buffer { buffer: *mut gpiod::gpiod_edge_event_buffer, }
-impl BufferInternal { +impl Buffer { /// Create a new edge event buffer. /// /// If capacity equals 0, it will be set to a default value of 64. If @@ -37,36 +36,6 @@ impl BufferInternal { pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { self.buffer } -} - -impl Drop for BufferInternal { - /// Free the edge event buffer and release all associated resources. - fn drop(&mut self) { - unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; - } -} - -/// Line edge events buffer -#[derive(Clone, Debug, Eq, PartialEq)] -pub struct Buffer { - ibuffer: Arc<BufferInternal>, -} - -impl Buffer { - /// Create a new edge event buffer. - /// - /// If capacity equals 0, it will be set to a default value of 64. If - /// capacity is larger than 1024, it will be limited to 1024. - pub fn new(capacity: usize) -> Result<Self> { - Ok(Self { - ibuffer: Arc::new(BufferInternal::new(capacity)?), - }) - } - - /// Private helper, Returns gpiod_edge_event_buffer - pub(crate) fn buffer(&self) -> *mut gpiod::gpiod_edge_event_buffer { - self.ibuffer.buffer() - }
/// Get the capacity of the event buffer. pub fn capacity(&self) -> usize { @@ -75,7 +44,7 @@ impl Buffer {
/// Read an event stored in the buffer. pub fn event(&self, index: u64) -> Resultedge::Event { - edge::Event::new(&self.ibuffer, index) + edge::Event::new(self, index) }
/// Get the number of events the buffer contains. @@ -88,3 +57,10 @@ impl Buffer { self.len() == 0 } } + +impl Drop for Buffer { + /// Free the edge event buffer and release all associated resources. + fn drop(&mut self) { + unsafe { gpiod::gpiod_edge_event_buffer_free(self.buffer) }; + } +} diff --git a/bindings/rust/libgpiod/src/line_request.rs b/bindings/rust/libgpiod/src/line_request.rs index 617efaa34d58..e4ff951aef29 100644 --- a/bindings/rust/libgpiod/src/line_request.rs +++ b/bindings/rust/libgpiod/src/line_request.rs @@ -218,7 +218,7 @@ impl Request { /// Get a number of edge events from a line request. /// /// This function will block if no event was queued for the line. - pub fn read_edge_events(&self, buffer: &edge::event::Buffer) -> Result<u32> { + pub fn read_edge_events(&self, buffer: &mut edge::event::Buffer) -> Result<u32> { let ret = unsafe { gpiod::gpiod_line_request_read_edge_event( self.request,
And here is an example where we get compilation error on the commented line:
fn main() -> Result<()> { let mut lsettings = line::Settings::new()?; let lconfig = line::Config::new()?; let offsets = vec![1, 2];
lsettings.set_prop(&[SettingVal::EdgeDetection(Some(Edge::Both))])?; lconfig.add_line_settings(&offsets, lsettings)?;
let path = "/dev/gpiochip0".to_string(); let chip = Chip::open(&path)?;
let rconfig = request::Config::new()?;
let mut buffer = edge::event::Buffer::new(2)?; let request = chip.request_lines(&rconfig, &lconfig)?;
loop { match request.wait_edge_event(None) { Err(x) => { println!("{:?}", x); return Err(Error::InvalidArguments); }
Ok(false) => { // This shouldn't happen as the call is blocking. panic!(); } Ok(true) => (), }
request.read_edge_events(&mut buffer)?; let event0 = buffer.event(0)?; let event1 = buffer.event(1)?;
println!("{:?}", (event0.line_offset(), event1.line_offset())); drop(event0);
// This fails to compile // request.read_edge_events(&mut buffer)?;
drop(event1); // This compiles fine request.read_edge_events(&mut buffer)?; } }