From 5ce27b16f1bcc613a49a140d167cae60213ae9ea Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Sep 2019 19:41:52 +0200 Subject: [PATCH] Remove context refernce from Chatlist See #476 aka a0b5e32f98eb7f9a4ae2a4432cbb353c2dec8374 for full rationale. Tl;dr it allows us to evolve the Rust API while keeping the FFI API identical. --- deltachat-ffi/src/lib.rs | 53 +++++++++++++++++++++++----------------- examples/repl/cmdline.rs | 2 +- examples/simple.rs | 2 +- src/chatlist.rs | 30 ++++++++--------------- 4 files changed, 43 insertions(+), 44 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 0b41cf4c9..5cd387981 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -405,19 +405,17 @@ pub unsafe extern "C" fn dc_maybe_network(context: *mut dc_context_t) { } #[no_mangle] -pub unsafe extern "C" fn dc_get_chatlist<'a>( +pub unsafe extern "C" fn dc_get_chatlist( context: *mut dc_context_t, flags: libc::c_int, query_str: *mut libc::c_char, query_id: u32, -) -> *mut dc_chatlist_t<'a> { +) -> *mut dc_chatlist_t { if context.is_null() { eprintln!("ignoring careless call to dc_get_chatlist()"); return ptr::null_mut(); } - let context = &*context; - let qs = if query_str.is_null() { None } else { @@ -425,7 +423,10 @@ pub unsafe extern "C" fn dc_get_chatlist<'a>( }; let qi = if query_id == 0 { None } else { Some(query_id) }; match chatlist::Chatlist::try_load(context, flags as usize, qs, qi) { - Ok(list) => Box::into_raw(Box::new(list)), + Ok(list) => { + let ffi_list = ChatlistWrapper { context, list }; + Box::into_raw(Box::new(ffi_list)) + } Err(_) => std::ptr::null_mut(), } } @@ -1633,8 +1634,20 @@ pub unsafe fn dc_array_is_independent( // dc_chatlist_t +/// FFI struct for [dc_chatlist_t] +/// +/// This is the structure behind [dc_chatlist_t] which is the opaque +/// structure representing a chatlist in the FFI API. It exists +/// because the FFI API has a refernce from the message to the +/// context, but the Rust API does not, so the FFI layer needs to glue +/// these together. +pub struct ChatlistWrapper { + context: *const dc_context_t, + list: chatlist::Chatlist, +} + #[no_mangle] -pub type dc_chatlist_t<'a> = chatlist::Chatlist<'a>; +pub type dc_chatlist_t = ChatlistWrapper; #[no_mangle] pub unsafe extern "C" fn dc_chatlist_unref(chatlist: *mut dc_chatlist_t) { @@ -1642,7 +1655,6 @@ pub unsafe extern "C" fn dc_chatlist_unref(chatlist: *mut dc_chatlist_t) { eprintln!("ignoring careless call to dc_chatlist_unref()"); return; } - Box::from_raw(chatlist); } @@ -1652,9 +1664,8 @@ pub unsafe extern "C" fn dc_chatlist_get_cnt(chatlist: *mut dc_chatlist_t) -> li eprintln!("ignoring careless call to dc_chatlist_get_cnt()"); return 0; } - - let list = &*chatlist; - list.len() as libc::size_t + let ffi_list = &*chatlist; + ffi_list.list.len() as libc::size_t } #[no_mangle] @@ -1666,9 +1677,8 @@ pub unsafe extern "C" fn dc_chatlist_get_chat_id( eprintln!("ignoring careless call to dc_chatlist_get_chat_id()"); return 0; } - - let list = &*chatlist; - list.get_chat_id(index as usize) + let ffi_list = &*chatlist; + ffi_list.list.get_chat_id(index as usize) } #[no_mangle] @@ -1680,9 +1690,8 @@ pub unsafe extern "C" fn dc_chatlist_get_msg_id( eprintln!("ignoring careless call to dc_chatlist_get_msg_id()"); return 0; } - - let list = &*chatlist; - list.get_msg_id(index as usize) + let ffi_list = &*chatlist; + ffi_list.list.get_msg_id(index as usize) } #[no_mangle] @@ -1701,8 +1710,10 @@ pub unsafe extern "C" fn dc_chatlist_get_summary( let ffi_chat = &*chat; Some(&ffi_chat.chat) }; - let list = &*chatlist; - let lot = list.get_summary(index as usize, maybe_chat); + let ffi_list = &*chatlist; + let lot = ffi_list + .list + .get_summary(&*ffi_list.context, index as usize, maybe_chat); Box::into_raw(Box::new(lot)) } @@ -1714,10 +1725,8 @@ pub unsafe extern "C" fn dc_chatlist_get_context( eprintln!("ignoring careless call to dc_chatlist_get_context()"); return ptr::null_mut(); } - - let list = &*chatlist; - - list.get_context() as *const _ + let ffi_list = &*chatlist; + ffi_list.context } // dc_chat_t diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 5e54343c1..aaf734713 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -592,7 +592,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E temp_subtitle, chat::get_fresh_msg_cnt(context, chat.get_id()), ); - let lot = chatlist.get_summary(i, Some(&chat)); + let lot = chatlist.get_summary(context, i, Some(&chat)); let statestr = if chat.is_archived() { " [Archived]" } else { diff --git a/examples/simple.rs b/examples/simple.rs index e5d31ea06..59df952c8 100644 --- a/examples/simple.rs +++ b/examples/simple.rs @@ -101,7 +101,7 @@ fn main() { let chats = Chatlist::try_load(&ctx, 0, None, None).unwrap(); for i in 0..chats.len() { - let summary = chats.get_summary(0, None); + let summary = chats.get_summary(&ctx, 0, None); let text1 = summary.get_text1(); let text2 = summary.get_text2(); println!("chat: {} - {:?} - {:?}", i, text1, text2,); diff --git a/src/chatlist.rs b/src/chatlist.rs index 03e959069..03165d65c 100644 --- a/src/chatlist.rs +++ b/src/chatlist.rs @@ -31,17 +31,12 @@ use crate::stock::StockMessage; /// first entry and only present on new messages, there is the rough idea that it can be optionally always /// present and sorted into the list by date. Rendering the deaddrop in the described way /// would not add extra work in the UI then. -pub struct Chatlist<'a> { - context: &'a Context, +pub struct Chatlist { /// Stores pairs of `chat_id, message_id` ids: Vec<(u32, u32)>, } -impl<'a> Chatlist<'a> { - pub fn get_context(&self) -> &Context { - self.context - } - +impl Chatlist { /// Get a list of chats. /// The list can be filtered by query parameters. /// @@ -85,7 +80,7 @@ impl<'a> Chatlist<'a> { /// `query_contact_id`: An optional contact ID for filtering the list. Only chats including this contact ID /// are returned. pub fn try_load( - context: &'a Context, + context: &Context, listflags: usize, query: Option<&str>, query_contact_id: Option, @@ -200,7 +195,7 @@ impl<'a> Chatlist<'a> { ids.push((DC_CHAT_ID_ARCHIVED_LINK, 0)); } - Ok(Chatlist { context, ids }) + Ok(Chatlist { ids }) } /// Find out the number of chats. @@ -247,7 +242,7 @@ impl<'a> Chatlist<'a> { /// - dc_lot_t::timestamp: the timestamp of the message. 0 if not applicable. /// - dc_lot_t::state: The state of the message as one of the DC_STATE_* constants (see #dc_msg_get_state()). // 0 if not applicable. - pub fn get_summary(&self, index: usize, chat: Option<&Chat>) -> Lot { + pub fn get_summary(&self, context: &Context, index: usize, chat: Option<&Chat>) -> Lot { // The summary is created by the chat, not by the last message. // This is because we may want to display drafts here or stuff as // "is typing". @@ -263,7 +258,7 @@ impl<'a> Chatlist<'a> { let chat = if let Some(chat) = chat { chat } else { - if let Ok(chat) = Chat::load_from_db(self.context, self.ids[index].0) { + if let Ok(chat) = Chat::load_from_db(context, self.ids[index].0) { chat_loaded = chat; &chat_loaded } else { @@ -275,11 +270,11 @@ impl<'a> Chatlist<'a> { let mut lastcontact = None; let lastmsg = if 0 != lastmsg_id { - if let Ok(lastmsg) = dc_msg_load_from_db(self.context, lastmsg_id) { + if let Ok(lastmsg) = dc_msg_load_from_db(context, lastmsg_id) { if lastmsg.from_id != 1 as libc::c_uint && (chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup) { - lastcontact = Contact::load_from_db(self.context, lastmsg.from_id).ok(); + lastcontact = Contact::load_from_db(context, lastmsg.from_id).ok(); } Some(lastmsg) @@ -294,14 +289,9 @@ impl<'a> Chatlist<'a> { ret.text2 = None; } else if lastmsg.is_none() || lastmsg.as_ref().unwrap().from_id == DC_CONTACT_ID_UNDEFINED { - ret.text2 = Some(self.context.stock_str(StockMessage::NoMessages).to_string()); + ret.text2 = Some(context.stock_str(StockMessage::NoMessages).to_string()); } else { - ret.fill( - &mut lastmsg.unwrap(), - chat, - lastcontact.as_ref(), - self.context, - ); + ret.fill(&mut lastmsg.unwrap(), chat, lastcontact.as_ref(), context); } ret