ffi: Use a generic wrapper for opaque Rust structs

src/keyboard.rs

@@ -5,6 +5,7 @@ use super::symbol;
 /// Gathers stuff defined in C or called by C
 pub mod c {
     use super::*;
+    use ::util::c;
     use ::util::c::{ as_cstr, into_cstring };
     
     use std::cell::RefCell;
@@ -13,10 +14,6 @@ pub mod c {
     use std::ptr;
     use std::rc::Rc;
 
-    // traits
-    
-    use std::borrow::ToOwned;
-
     
     // The following defined in C
     #[no_mangle]
@@ -24,44 +21,7 @@ pub mod c {
         fn eek_keysym_from_name(name: *const c_char) -> u32;
     }
 
-    /// The wrapped structure for KeyState suitable for handling in C
+    pub type CKeyState = c::Wrapped<KeyState>;
-    /// Since C doesn't respect borrowing rules,
-    /// RefCell will enforce them dynamically (only 1 writer/many readers)
-    /// Rc is implied and will ensure timely dropping
-    #[repr(transparent)]
-    pub struct CKeyState(*const RefCell<KeyState>);
-    
-    impl Clone for CKeyState {
-        fn clone(&self) -> Self {
-            CKeyState(self.0.clone())
-        }
-    }
-
-    impl CKeyState {
-        pub fn wrap(state: Rc<RefCell<KeyState>>) -> CKeyState {
-            CKeyState(Rc::into_raw(state))
-        }
-        pub fn unwrap(self) -> Rc<RefCell<KeyState>> {
-            unsafe { Rc::from_raw(self.0) }
-        }
-        fn to_owned(self) -> KeyState {
-            let rc = self.unwrap();
-            let state = rc.borrow().to_owned();
-            Rc::into_raw(rc); // Prevent dropping
-            state
-        }
-        fn borrow_mut<F, T>(self, f: F) -> T where F: FnOnce(&mut KeyState) -> T {
-            let rc = self.unwrap();
-            let ret = {
-                let mut state = rc.borrow_mut();
-                f(&mut state)
-            };
-            Rc::into_raw(rc); // Prevent dropping
-            ret
-        }
-    }
-
-    // TODO: unwrapping
 
     // The following defined in Rust. TODO: wrap naked pointers to Rust data inside RefCells to prevent multiple writers
     
@@ -84,7 +44,7 @@ pub mod c {
     #[no_mangle]
     pub extern "C"
     fn squeek_key_free(key: CKeyState) {
-        key.unwrap(); // reference dropped
+        unsafe { key.unwrap() }; // reference dropped
     }
     
     #[no_mangle]
@@ -97,7 +57,9 @@ pub mod c {
     #[no_mangle]
     pub extern "C"
     fn squeek_key_set_pressed(key: CKeyState, pressed: u32) {
-        key.borrow_mut(|key| key.pressed = pressed != 0);
+        let key = key.clone_ref();
+        let mut key = key.borrow_mut();
+        key.pressed = pressed != 0;
     }
     
     #[no_mangle]
@@ -109,7 +71,9 @@ pub mod c {
     #[no_mangle]
     pub extern "C"
     fn squeek_key_set_locked(key: CKeyState, locked: u32) {
-        key.borrow_mut(|key| key.locked = locked != 0);
+        let key = key.clone_ref();
+        let mut key = key.borrow_mut();
+        key.locked = locked != 0;
     }
     
     #[no_mangle]
@@ -121,7 +85,9 @@ pub mod c {
     #[no_mangle]
     pub extern "C"
     fn squeek_key_set_keycode(key: CKeyState, code: u32) {
-        key.borrow_mut(|key| key.keycode = code);
+        let key = key.clone_ref();
+        let mut key = key.borrow_mut();
+        key.keycode = code;
     }
     
     // TODO: this will receive data from the filesystem,
@@ -180,38 +146,38 @@ pub mod c {
                 None
             });
         
+        let key = key.clone_ref();
+        let mut key = key.borrow_mut();
 
-        key.borrow_mut(|key| {
+        if let Some(_) = key.symbol {
-            if let Some(_) = key.symbol {
+            eprintln!("Key {:?} already has a symbol defined", text);
-                eprintln!("Key {:?} already has a symbol defined", text);
+            return;
-                return;
+        }
-            }
 
-            key.symbol = Some(match element.to_bytes() {
+        key.symbol = Some(match element.to_bytes() {
-                b"symbol" => Symbol {
+            b"symbol" => Symbol {
-                    action: Action::Submit {
+                action: Action::Submit {
-                        text: text,
+                    text: text,
-                        keys: Vec::new(),
+                    keys: Vec::new(),
-                    },
-                    label: label,
-                    tooltip: tooltip,
                 },
-                _ => panic!("unsupported element type {:?}", element),
+                label: label,
-            });
+                tooltip: tooltip,
+            },
+            _ => panic!("unsupported element type {:?}", element),
         });
     }
 
     #[no_mangle]
     pub extern "C"
     fn squeek_key_get_symbol(key: CKeyState) -> *const symbol::Symbol {
-        key.borrow_mut(|key| {
+        let key = key.clone_ref();
-            match key.symbol {
+        let key = key.borrow();
-                // This pointer stays after the function exits,
+        match key.symbol {
-                // so it must reference borrowed data and not any copy
+            // This pointer stays after the function exits,
-                Some(ref symbol) => symbol as *const symbol::Symbol,
+            // so it must reference borrowed data and not any copy
-                None => ptr::null(),
+            Some(ref symbol) => symbol as *const symbol::Symbol,
-            }
+            None => ptr::null(),
-        })
+        }
     }
 
     #[no_mangle]

src/layout.rs

@@ -315,9 +315,8 @@ pub mod c {
         state: ::keyboard::c::CKeyState,
     ) -> u32 {
         let button = unsafe { &*button };
-        let state = state.unwrap();
+        let state = state.clone_ref();
         let equal = Rc::ptr_eq(&button.state, &state);
-        Rc::into_raw(state); // Prevent dropping
         equal as u32
     }
     
@@ -418,13 +417,10 @@ pub mod c {
             needle: ::keyboard::c::CKeyState,
         ) -> ButtonPlace {
             let view = unsafe { &*view };
-            let state = needle.unwrap();
+            let state = needle.clone_ref();
             
             let paths = ::layout::procedures::find_key_paths(view, &state);
 
-            // Iterators used up, can turn the reference back into pointer
-            Rc::into_raw(state);
-
             // Can only return 1 entry back to C
             let (row, button) = match paths.get(0) {
                 Some((row, button)) => (

src/util.rs

@@ -1,7 +1,13 @@
 pub mod c {
+    use std::cell::RefCell;
     use std::ffi::{ CStr, CString };
     use std::os::raw::c_char;
+    use std::rc::Rc;
     use std::str::Utf8Error;
+
+    // traits
+    
+    use std::borrow::ToOwned;
     
     #[allow(dead_code)]
     pub fn as_str(s: &*const c_char) -> Result<Option<&str>, Utf8Error> {
@@ -47,4 +53,52 @@ pub mod c {
             assert_eq!(as_str(&ptr::null()), Ok(None))
         }
     }
+
+    /// Wraps structures to pass them safely to/from C
+    /// Since C doesn't respect borrowing rules,
+    /// RefCell will enforce them dynamically (only 1 writer/many readers)
+    /// Rc is implied and will ensure timely dropping
+    #[repr(transparent)]
+    pub struct Wrapped<T: Clone>(*const RefCell<T>);
+
+    // It would be nice to implement `Borrow`
+    // directly on the raw pointer to avoid one conversion call,
+    // but the `borrow()` call needs to extract a `Rc`,
+    // and at the same time return a reference to it (`std::cell::Ref`)
+    // to take advantage of `Rc<RefCell>::borrow()` runtime checks.
+    // Unfortunately, that needs a `Ref` struct with self-referential fields,
+    // which is a bit too complex for now.
+
+    impl<T: Clone> Wrapped<T> {
+        pub fn wrap(state: Rc<RefCell<T>>) -> Wrapped<T> {
+            Wrapped(Rc::into_raw(state))
+        }
+        /// Extracts the reference to the data.
+        /// It may cause problems if attempted in more than one place
+        pub unsafe fn unwrap(self) -> Rc<RefCell<T>> {
+            Rc::from_raw(self.0)
+        }
+        
+        /// Creates a new Rc reference to the same data
+        pub fn clone_ref(&self) -> Rc<RefCell<T>> {
+            // A bit dangerous: the Rc may be in use elsewhere
+            let used_rc = unsafe { Rc::from_raw(self.0) };
+            let rc = used_rc.clone();
+            Rc::into_raw(used_rc); // prevent dropping the original reference
+            rc
+        }
+
+        /// Create a copy of the underlying data
+        pub fn to_owned(&self) -> T {
+            let rc = self.clone_ref();
+            let r = rc.borrow();
+            r.to_owned()
+        }
+    }
+    
+    impl<T: Clone> Clone for Wrapped<T> {
+        fn clone(&self) -> Wrapped<T> {
+            Wrapped::wrap(self.clone_ref())
+        }
+    }
 }