Hi Björn,
I have bounced (mutt's feature) the initial emails to your and other email ids that Miguel added. The patches should be in your inbox now.
On 20-10-22, 13:29, Björn Roy Baron wrote:
At https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b... and elsewhere you might want to use `CStr::from_ptr(version)`. This does the `strlen` call for you and you can convert it to an `&str` using `.to_str()`.
At https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b... you could use `CString` and use the `.as_ptr()` method to get a null-terminated string. Not sure if it would be nicer that what you currently have though.
These two were nice. Thanks.
At https://github.com/vireshk/libgpiod/blob/3e7fb99173856a3995360fc3fad51220c4b... the lifetimes are unclear. Is Event allowed to outlive the buffer? Can you add a lifetime annotation like fn event_clone<'a>(event: &Event<'a>) -> Result<Event<'a>> if it isn't allowed to outlive the buffer or fn event_clone<'a, 'b>(event: &Event<'a>) -> Result<Event<'b>> if it is allowed to outlive the buffer. I'm not sure which of the two the lifetime elision rules cause the current code to be equivalent of, but even if it is correct, explicitly stating the lifetime here is clearer IMHO.
An Event created using Event::new() can't outlive the buffer, though an Event created using Event::event_clone() can.
I tried to play around it based on your suggestion and here is the diff, does it make sense ?
diff --git a/bindings/rust/libgpiod/src/edge_event.rs b/bindings/rust/libgpiod/src/edge_event.rs index b36c23601bb4..0d328ebb2b03 100644 --- a/bindings/rust/libgpiod/src/edge_event.rs +++ b/bindings/rust/libgpiod/src/edge_event.rs @@ -33,7 +33,7 @@ pub struct Event<'b> {
impl<'b> Event<'b> { /// Get an event stored in the buffer. - pub(crate) fn new(buffer: &'b Buffer, index: usize) -> Result<Self> { + pub(crate) fn new(buffer: &'b Buffer, index: usize) -> Result<Event<'b>> { // SAFETY: The `gpiod_edge_event` returned by libgpiod is guaranteed to live as long // as the `struct Event`. let event = unsafe { @@ -52,22 +52,6 @@ impl<'b> Event<'b> { }) }
- pub fn event_clone(event: &Event) -> Result<Self> { - // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. - let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; - if event.is_null() { - return Err(Error::OperationFailed( - OperationType::EdgeEventCopy, - Errno::last(), - )); - } - - Ok(Self { - buffer: None, - event, - }) - } - /// Get the event type. pub fn event_type(&self) -> Result<EdgeKind> { // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. @@ -105,6 +89,27 @@ impl<'b> Event<'b> { } }
+impl<'e, 'b> Event<'e> { + pub fn event_clone(event: &Event<'b>) -> Result<Event<'e>> + where + 'e: 'b, + { + // SAFETY: `gpiod_edge_event` is guaranteed to be valid here. + let event = unsafe { gpiod::gpiod_edge_event_copy(event.event) }; + if event.is_null() { + return Err(Error::OperationFailed( + OperationType::EdgeEventCopy, + Errno::last(), + )); + } + + Ok(Self { + buffer: None, + event, + }) + } +} +