diff mbox series

[libgpiod,v3,3/3] bindings: rust: allow cloning line::InfoRef -> line::Info

Message ID 20231003-rust-line-info-soundness-v3-3-555ba21b4632@linaro.org
State New
Headers show
Series bindings: rust: fix modeling of line_info lifetimes | expand

Commit Message

Erik Schilling Oct. 3, 2023, 9:39 a.m. UTC
While one would usually use the ToOwned [1] contract in rust, libgpipd's
API only allows copying that may fail.

Thus, we cannot implement the existing trait and roll our own method. I
went with `try_clone` since that seems to be used in similar cases across
the `std` crate [2].

It also closes the gap of not having any way to clone owned instances.
Though - again - not through the Clone trait which may not fail [3].

[1] https://doc.rust-lang.org/std/borrow/trait.ToOwned.html
[2] https://doc.rust-lang.org/std/index.html?search=try_clone
[3] https://doc.rust-lang.org/std/clone/trait.Clone.html

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Erik Schilling <erik.schilling@linaro.org>
---
 bindings/rust/libgpiod/src/lib.rs         |  1 +
 bindings/rust/libgpiod/src/line_info.rs   | 16 ++++++++++
 bindings/rust/libgpiod/tests/line_info.rs | 53 +++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+)
diff mbox series

Patch

diff --git a/bindings/rust/libgpiod/src/lib.rs b/bindings/rust/libgpiod/src/lib.rs
index 3acc98c..fd95ed2 100644
--- a/bindings/rust/libgpiod/src/lib.rs
+++ b/bindings/rust/libgpiod/src/lib.rs
@@ -74,6 +74,7 @@  pub enum OperationType {
     LineConfigSetOutputValues,
     LineConfigGetOffsets,
     LineConfigGetSettings,
+    LineInfoCopy,
     LineRequestReconfigLines,
     LineRequestGetVal,
     LineRequestGetValSubset,
diff --git a/bindings/rust/libgpiod/src/line_info.rs b/bindings/rust/libgpiod/src/line_info.rs
index 2148789..bd290f6 100644
--- a/bindings/rust/libgpiod/src/line_info.rs
+++ b/bindings/rust/libgpiod/src/line_info.rs
@@ -58,6 +58,22 @@  impl InfoRef {
         self as *const _ as *mut _
     }
 
+    /// Clones the line info object.
+    pub fn try_clone(&self) -> Result<Info> {
+        // SAFETY: `gpiod_line_info` is guaranteed to be valid here.
+        let copy = unsafe { gpiod::gpiod_line_info_copy(self.as_raw_ptr()) };
+        if copy.is_null() {
+            return Err(Error::OperationFailed(
+                crate::OperationType::LineInfoCopy,
+                errno::errno(),
+            ));
+        }
+
+        // SAFETY: The copy succeeded, we are the owner and stop using the
+        // pointer after this.
+        Ok(unsafe { Info::from_raw(copy) })
+    }
+
     /// Get the offset of the line within the GPIO chip.
     ///
     /// The offset uniquely identifies the line on the chip. The combination of the chip and offset
diff --git a/bindings/rust/libgpiod/tests/line_info.rs b/bindings/rust/libgpiod/tests/line_info.rs
index ce66a60..d02c9ea 100644
--- a/bindings/rust/libgpiod/tests/line_info.rs
+++ b/bindings/rust/libgpiod/tests/line_info.rs
@@ -19,6 +19,10 @@  mod line_info {
     const NGPIO: usize = 8;
 
     mod properties {
+        use std::thread;
+
+        use libgpiod::{line, request};
+
         use super::*;
 
         #[test]
@@ -271,5 +275,54 @@  mod line_info {
             assert!(info.is_debounced());
             assert_eq!(info.debounce_period(), Duration::from_millis(100));
         }
+
+        fn generate_line_event(chip: &Chip) {
+            let mut line_config = line::Config::new().unwrap();
+            line_config
+                .add_line_settings(&[0], line::Settings::new().unwrap())
+                .unwrap();
+
+            let mut request = chip
+                .request_lines(Some(&request::Config::new().unwrap()), &line_config)
+                .unwrap();
+
+            let mut new_line_config = line::Config::new().unwrap();
+            let mut settings = line::Settings::new().unwrap();
+            settings.set_direction(Direction::Output).unwrap();
+            new_line_config.add_line_settings(&[0], settings).unwrap();
+            request.reconfigure_lines(&new_line_config).unwrap();
+        }
+
+        #[test]
+        fn ownership() {
+            let sim = Sim::new(Some(1), None, false).unwrap();
+            sim.set_line_name(0, "Test line").unwrap();
+            sim.enable().unwrap();
+
+            let chip = Chip::open(&sim.dev_path()).unwrap();
+
+            // start watching line
+            chip.watch_line_info(0).unwrap();
+
+            generate_line_event(&chip);
+
+            // read generated event
+            let event = chip.read_info_event().unwrap();
+            let info = event.line_info().unwrap();
+            assert_eq!(info.name().unwrap(), "Test line");
+
+            // clone info and move to separate thread
+            let info = info.try_clone().unwrap();
+
+            // drop the original event with the associated line_info
+            drop(event);
+
+            // assert that we can still read the name
+            thread::scope(|s| {
+                s.spawn(move || {
+                    assert_eq!(info.name().unwrap(), "Test line");
+                });
+            });
+        }
     }
 }