From 878b7ed18e0e7541697b780fbc0b7b0350267ac7 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 28 Aug 2019 20:16:28 +0000 Subject: [PATCH] ffi: Use a generic wrapper for opaque Rust structs --- src/keyboard.rs | 104 ++++++++++++++++-------------------------------- src/layout.rs | 8 +--- src/util.rs | 54 +++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 75 deletions(-) diff --git a/src/keyboard.rs b/src/keyboard.rs index 5c7b203f..c2356b00 100644 --- a/src/keyboard.rs +++ b/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 - /// 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); - - impl Clone for CKeyState { - fn clone(&self) -> Self { - CKeyState(self.0.clone()) - } - } - - impl CKeyState { - pub fn wrap(state: Rc>) -> CKeyState { - CKeyState(Rc::into_raw(state)) - } - pub fn unwrap(self) -> Rc> { - 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(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 + pub type CKeyState = c::Wrapped; // 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 { - eprintln!("Key {:?} already has a symbol defined", text); - return; - } + if let Some(_) = key.symbol { + eprintln!("Key {:?} already has a symbol defined", text); + return; + } - key.symbol = Some(match element.to_bytes() { - b"symbol" => Symbol { - action: Action::Submit { - text: text, - keys: Vec::new(), - }, - label: label, - tooltip: tooltip, + key.symbol = Some(match element.to_bytes() { + b"symbol" => Symbol { + action: Action::Submit { + text: text, + keys: Vec::new(), }, - _ => 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| { - match key.symbol { - // This pointer stays after the function exits, - // so it must reference borrowed data and not any copy - Some(ref symbol) => symbol as *const symbol::Symbol, - None => ptr::null(), - } - }) + let key = key.clone_ref(); + let key = key.borrow(); + match key.symbol { + // This pointer stays after the function exits, + // so it must reference borrowed data and not any copy + Some(ref symbol) => symbol as *const symbol::Symbol, + None => ptr::null(), + } } #[no_mangle] diff --git a/src/layout.rs b/src/layout.rs index b21d5d23..9a2cec85 100644 --- a/src/layout.rs +++ b/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)) => ( diff --git a/src/util.rs b/src/util.rs index 53290e7a..de3b7d33 100644 --- a/src/util.rs +++ b/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, 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(*const RefCell); + + // 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::borrow()` runtime checks. + // Unfortunately, that needs a `Ref` struct with self-referential fields, + // which is a bit too complex for now. + + impl Wrapped { + pub fn wrap(state: Rc>) -> Wrapped { + 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> { + Rc::from_raw(self.0) + } + + /// Creates a new Rc reference to the same data + pub fn clone_ref(&self) -> Rc> { + // 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 Clone for Wrapped { + fn clone(&self) -> Wrapped { + Wrapped::wrap(self.clone_ref()) + } + } }