From 8326bd7016aa7a3d4407f4acda22a695f49783a8 Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 31 Jul 2019 09:37:09 +0000 Subject: [PATCH 1/4] rust: Create a root file for modules The new `lib.rs` file is created to refer to all modules written in Rust. This way, only one `rustc` call is needed to compile an arbitrary amount of modules. It also converges with the way crates are structured. --- src/imservice.rs | 9 ++++----- src/lib.rs | 3 +++ src/meson.build | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) create mode 100644 src/lib.rs diff --git a/src/imservice.rs b/src/imservice.rs index b59dad66..d55f2ba7 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -1,11 +1,10 @@ -#[macro_use] -mod bitflags; - use std::boxed::Box; use std::ffi::CString; use std::num::Wrapping; use std::string::String; +use super::bitflags; + /// Gathers stuff defined in C or called by C pub mod c { use super::*; @@ -221,7 +220,7 @@ pub enum ContentPurpose { impl ContentPurpose { fn from_num(num: u32) -> Option { - use ContentPurpose::*; + use self::ContentPurpose::*; match num { 0 => Some(Normal), 1 => Some(Alpha), @@ -241,7 +240,7 @@ impl ContentPurpose { } } fn as_num(self: &ContentPurpose) -> u32 { - use ContentPurpose::*; + use self::ContentPurpose::*; match self { Normal => 0, Alpha => 1, diff --git a/src/lib.rs b/src/lib.rs new file mode 100644 index 00000000..499a1a03 --- /dev/null +++ b/src/lib.rs @@ -0,0 +1,3 @@ +#[macro_use] +mod bitflags; +mod imservice; diff --git a/src/meson.build b/src/meson.build index 4e5e07ab..2d7fa5c3 100644 --- a/src/meson.build +++ b/src/meson.build @@ -69,7 +69,7 @@ deps = [ # Replacement for eekboard-server rslib = static_library( 'rslib', - sources: ['imservice.rs'], + sources: ['lib.rs'], rust_crate_type: 'staticlib' ) From 58d01bf502df014dac5d8c6d69f24b2860c2668d Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 31 Jul 2019 09:50:26 +0000 Subject: [PATCH 2/4] imservice: Use discriminants in enums --- src/imservice.rs | 63 ++++++++++++++++-------------------------------- 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/src/imservice.rs b/src/imservice.rs index d55f2ba7..39e4d0e3 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -155,7 +155,7 @@ pub mod c { eekboard_context_service_set_hint_purpose( imservice.ui_manager, imservice.current.content_hint.bits(), - imservice.current.content_purpose.as_num()); + imservice.current.content_purpose.clone() as u32); } else { eekboard_context_service_hide_keyboard(imservice.ui_manager); } @@ -200,22 +200,26 @@ bitflags!{ } /// Map to `text_input_unstable_v3.content_purpose` values +/// +/// ``` +/// assert_eq!(ContentPurpose::Alpha as u32, 0); +/// ``` #[derive(Debug, Clone)] pub enum ContentPurpose { - Normal, - Alpha, - Digits, - Number, - Phone, - Url, - Email, - Name, - Password, - Pin, - Date, - Time, - Datetime, - Terminal, + Normal = 0, + Alpha = 1, + Digits = 2, + Number = 3, + Phone = 4, + Url = 5, + Email = 6, + Name = 7, + Password = 8, + Pin = 9, + Date = 10, + Time = 11, + Datetime = 12, + Terminal = 13, } impl ContentPurpose { @@ -239,32 +243,13 @@ impl ContentPurpose { _ => None, } } - fn as_num(self: &ContentPurpose) -> u32 { - use self::ContentPurpose::*; - match self { - Normal => 0, - Alpha => 1, - Digits => 2, - Number => 3, - Phone => 4, - Url => 5, - Email => 6, - Name => 7, - Password => 8, - Pin => 9, - Date => 10, - Time => 11, - Datetime => 12, - Terminal => 13, - } - } } /// Map to `text_input_unstable_v3.change_cause` values #[derive(Debug, Clone)] pub enum ChangeCause { - InputMethod, - Other, + InputMethod = 0, + Other = 1, } impl ChangeCause { @@ -275,12 +260,6 @@ impl ChangeCause { _ => None } } - pub fn as_num(&self) -> u32 { - match &self { - ChangeCause::InputMethod => 0, - ChangeCause::Other => 1, - } - } } /// Describes the desired state of the input method as requested by the server From 98a2e33d78114d9ba67083b44a02a6fbb979d09d Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 31 Jul 2019 09:59:25 +0000 Subject: [PATCH 3/4] imservice: Use TryFrom for u32->enum conversions --- src/imservice.rs | 62 ++++++++++++++++++++++++++++-------------------- 1 file changed, 36 insertions(+), 26 deletions(-) diff --git a/src/imservice.rs b/src/imservice.rs index 39e4d0e3..b48d9ae7 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -5,6 +5,10 @@ use std::string::String; use super::bitflags; +// Traits +use std::convert::TryFrom; + + /// Gathers stuff defined in C or called by C pub mod c { use super::*; @@ -108,8 +112,8 @@ pub mod c { }) }, content_purpose: { - ContentPurpose::from_num(purpose).unwrap_or_else(|| { - eprintln!("Warning: Received invalid purpose flags"); + ContentPurpose::try_from(purpose).unwrap_or_else(|_e| { + eprintln!("Warning: Received invalid purpose value"); ContentPurpose::Normal }) }, @@ -126,8 +130,8 @@ pub mod c { let imservice = &mut *imservice; imservice.pending = IMProtocolState { text_change_cause: { - ChangeCause::from_num(cause).unwrap_or_else(|| { - eprintln!("Warning: received invalid cause flags"); + ChangeCause::try_from(cause).unwrap_or_else(|_e| { + eprintln!("Warning: received invalid cause value"); ChangeCause::InputMethod }) }, @@ -222,25 +226,28 @@ pub enum ContentPurpose { Terminal = 13, } -impl ContentPurpose { - fn from_num(num: u32) -> Option { +impl TryFrom for ContentPurpose { + // There's only one way to fail: number not in protocol, + // so no special error type is needed + type Error = (); + fn try_from(num: u32) -> Result { use self::ContentPurpose::*; match num { - 0 => Some(Normal), - 1 => Some(Alpha), - 2 => Some(Digits), - 3 => Some(Number), - 4 => Some(Phone), - 5 => Some(Url), - 6 => Some(Email), - 7 => Some(Name), - 8 => Some(Password), - 9 => Some(Pin), - 10 => Some(Date), - 11 => Some(Time), - 12 => Some(Datetime), - 13 => Some(Terminal), - _ => None, + 0 => Ok(Normal), + 1 => Ok(Alpha), + 2 => Ok(Digits), + 3 => Ok(Number), + 4 => Ok(Phone), + 5 => Ok(Url), + 6 => Ok(Email), + 7 => Ok(Name), + 8 => Ok(Password), + 9 => Ok(Pin), + 10 => Ok(Date), + 11 => Ok(Time), + 12 => Ok(Datetime), + 13 => Ok(Terminal), + _ => Err(()), } } } @@ -252,12 +259,15 @@ pub enum ChangeCause { Other = 1, } -impl ChangeCause { - pub fn from_num(num: u32) -> Option { +impl TryFrom for ChangeCause { + // There's only one way to fail: number not in protocol, + // so no special error type is needed + type Error = (); + fn try_from(num: u32) -> Result { match num { - 0 => Some(ChangeCause::InputMethod), - 1 => Some(ChangeCause::Other), - _ => None + 0 => Ok(ChangeCause::InputMethod), + 1 => Ok(ChangeCause::Other), + _ => Err(()) } } } From fa31f8eee16ebff20c6dc499a316ec1d1bdfe0ea Mon Sep 17 00:00:00 2001 From: Dorota Czaplejewicz Date: Wed, 31 Jul 2019 10:43:39 +0000 Subject: [PATCH 4/4] imservice: Check pointer validity --- src/imservice.rs | 54 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/src/imservice.rs b/src/imservice.rs index b48d9ae7..5979f86d 100644 --- a/src/imservice.rs +++ b/src/imservice.rs @@ -61,9 +61,9 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_input_method_activate(imservice: *mut IMService, - _im: *const InputMethod) + im: *const InputMethod) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); imservice.preedit_string = String::new(); imservice.pending = IMProtocolState { active: true, @@ -74,9 +74,9 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_input_method_deactivate(imservice: *mut IMService, - _im: *const InputMethod) + im: *const InputMethod) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { active: false, ..imservice.pending.clone() @@ -86,10 +86,10 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_surrounding_text(imservice: *mut IMService, - _im: *const InputMethod, + im: *const InputMethod, text: *const c_char, cursor: u32, _anchor: u32) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { surrounding_text: into_cstring(text).expect("Received invalid string"), surrounding_cursor: cursor, @@ -100,10 +100,10 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_content_type(imservice: *mut IMService, - _im: *const InputMethod, + im: *const InputMethod, hint: u32, purpose: u32) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { content_hint: { ContentHint::from_bits(hint).unwrap_or_else(|| { @@ -124,10 +124,10 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_text_change_cause(imservice: *mut IMService, - _im: *const InputMethod, + im: *const InputMethod, cause: u32) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); imservice.pending = IMProtocolState { text_change_cause: { ChangeCause::try_from(cause).unwrap_or_else(|_e| { @@ -142,9 +142,9 @@ pub mod c { #[no_mangle] pub unsafe extern "C" fn imservice_handle_commit_state(imservice: *mut IMService, - _im: *const InputMethod) + im: *const InputMethod) { - let imservice = &mut *imservice; + let imservice = check_imservice(imservice, im).unwrap(); let active_changed = imservice.current.active ^ imservice.pending.active; imservice.serial += Wrapping(1u32); @@ -171,10 +171,9 @@ pub mod c { fn imservice_handle_unavailable(imservice: *mut IMService, im: *mut InputMethod) { + let imservice = check_imservice(imservice, im).unwrap(); imservice_destroy_im(im); - let imservice = &mut *imservice; - // no need to care about proper double-buffering, // the keyboard is already decommissioned imservice.current.active = false; @@ -183,6 +182,31 @@ pub mod c { } // FIXME: destroy and deallocate + + // Helpers + + /// Convenience method for referencing the IMService raw pointer, + /// and for verifying that the input method passed along + /// matches the one in the `imservice`. + /// + /// The lifetime of the returned value is 'static, + /// due to the fact that the lifetime of a raw pointer is undefined. + /// Care must be take + /// not to exceed the lifetime of the pointer with the reference, + /// especially not to store it. + fn check_imservice(imservice: *mut IMService, im: *const InputMethod) + -> Result<&'static mut IMService, &'static str> + { + if imservice.is_null() { + return Err("Null imservice pointer"); + } + let imservice: &mut IMService = unsafe { &mut *imservice }; + if im == imservice.im { + Ok(imservice) + } else { + Err("Imservice doesn't contain the input method") + } + } } @@ -298,7 +322,7 @@ impl Default for IMProtocolState { pub struct IMService { /// Owned reference (still created and destroyed in C) - im: *const c::InputMethod, + pub im: *const c::InputMethod, /// Unowned reference. Be careful, it's shared with C at large ui_manager: *const c::UIManager,