From 8302d6833d8fd94fd064737c0b5cb98ee6e67892 Mon Sep 17 00:00:00 2001 From: Floris Bruynooghe Date: Wed, 11 Sep 2019 19:04:29 +0200 Subject: [PATCH] Remove context ref from Chat struct Leaving the C API untouched. See #476 aka a0b5e32f98eb7f9a4ae2a4432cbb353c2dec8374 for more extensive rationale. --- deltachat-ffi/src/lib.rs | 118 +++++++++++++++++++-------------------- examples/repl/cmdline.rs | 4 +- src/chat.rs | 73 +++++++++++------------- src/chatlist.rs | 2 +- src/dc_mimefactory.rs | 7 ++- src/dc_receive_imf.rs | 2 +- src/message.rs | 2 +- 7 files changed, 99 insertions(+), 109 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 047654966..0b41cf4c9 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -780,19 +780,17 @@ pub unsafe extern "C" fn dc_search_msgs( } #[no_mangle] -pub unsafe extern "C" fn dc_get_chat<'a>( - context: *mut dc_context_t, - chat_id: u32, -) -> *mut dc_chat_t<'a> { +pub unsafe extern "C" fn dc_get_chat(context: *mut dc_context_t, chat_id: u32) -> *mut dc_chat_t { if context.is_null() { eprintln!("ignoring careless call to dc_get_chat()"); return ptr::null_mut(); } - let context = &*context; - match chat::Chat::load_from_db(context, chat_id) { - Ok(chat) => Box::into_raw(Box::new(chat)), + Ok(chat) => { + let ffi_chat = ChatWrapper { context, chat }; + Box::into_raw(Box::new(ffi_chat)) + } Err(_) => std::ptr::null_mut(), } } @@ -1688,20 +1686,23 @@ pub unsafe extern "C" fn dc_chatlist_get_msg_id( } #[no_mangle] -pub unsafe extern "C" fn dc_chatlist_get_summary<'a>( - chatlist: *mut dc_chatlist_t<'a>, +pub unsafe extern "C" fn dc_chatlist_get_summary( + chatlist: *mut dc_chatlist_t, index: libc::size_t, - chat: *mut dc_chat_t<'a>, + chat: *mut dc_chat_t, ) -> *mut dc_lot_t { if chatlist.is_null() { eprintln!("ignoring careless call to dc_chatlist_get_summary()"); return ptr::null_mut(); } - - let chat = if chat.is_null() { None } else { Some(&*chat) }; + let maybe_chat = if chat.is_null() { + None + } else { + let ffi_chat = &*chat; + Some(&ffi_chat.chat) + }; let list = &*chatlist; - - let lot = list.get_summary(index as usize, chat); + let lot = list.get_summary(index as usize, maybe_chat); Box::into_raw(Box::new(lot)) } @@ -1721,8 +1722,20 @@ pub unsafe extern "C" fn dc_chatlist_get_context( // dc_chat_t +/// FFI struct for [dc_chat_t] +/// +/// This is the structure behind [dc_chat_t] which is the opaque +/// structure representing a chat 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 ChatWrapper { + context: *const dc_context_t, + chat: chat::Chat, +} + #[no_mangle] -pub type dc_chat_t<'a> = chat::Chat<'a>; +pub type dc_chat_t = ChatWrapper; #[no_mangle] pub unsafe extern "C" fn dc_chat_unref(chat: *mut dc_chat_t) { @@ -1740,10 +1753,8 @@ pub unsafe extern "C" fn dc_chat_get_id(chat: *mut dc_chat_t) -> u32 { eprintln!("ignoring careless call to dc_chat_get_id()"); return 0; } - - let chat = &*chat; - - chat.get_id() + let ffi_chat = &*chat; + ffi_chat.chat.get_id() } #[no_mangle] @@ -1752,10 +1763,8 @@ pub unsafe extern "C" fn dc_chat_get_type(chat: *mut dc_chat_t) -> libc::c_int { eprintln!("ignoring careless call to dc_chat_get_type()"); return 0; } - - let chat = &*chat; - - chat.get_type() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.get_type() as libc::c_int } #[no_mangle] @@ -1764,10 +1773,8 @@ pub unsafe extern "C" fn dc_chat_get_name(chat: *mut dc_chat_t) -> *mut libc::c_ eprintln!("ignoring careless call to dc_chat_get_name()"); return dc_strdup(ptr::null()); } - - let chat = &*chat; - - chat.get_name().strdup() + let ffi_chat = &*chat; + ffi_chat.chat.get_name().strdup() } #[no_mangle] @@ -1776,10 +1783,8 @@ pub unsafe extern "C" fn dc_chat_get_subtitle(chat: *mut dc_chat_t) -> *mut libc eprintln!("ignoring careless call to dc_chat_get_subtitle()"); return dc_strdup(ptr::null()); } - - let chat = &*chat; - - chat.get_subtitle().strdup() + let ffi_chat = &*chat; + ffi_chat.chat.get_subtitle(&*ffi_chat.context).strdup() } #[no_mangle] @@ -1788,10 +1793,8 @@ pub unsafe extern "C" fn dc_chat_get_profile_image(chat: *mut dc_chat_t) -> *mut eprintln!("ignoring careless call to dc_chat_get_profile_image()"); return ptr::null_mut(); // NULL explicitly defined as "no image" } - - let chat = &*chat; - - match chat.get_profile_image() { + let ffi_chat = &*chat; + match ffi_chat.chat.get_profile_image(&*ffi_chat.context) { Some(p) => p.to_str().unwrap().to_string().strdup(), None => ptr::null_mut(), } @@ -1803,10 +1806,8 @@ pub unsafe extern "C" fn dc_chat_get_color(chat: *mut dc_chat_t) -> u32 { eprintln!("ignoring careless call to dc_chat_get_color()"); return 0; } - - let chat = &*chat; - - chat.get_color() + let ffi_chat = &*chat; + ffi_chat.chat.get_color(&*ffi_chat.context) } #[no_mangle] @@ -1815,10 +1816,8 @@ pub unsafe extern "C" fn dc_chat_get_archived(chat: *mut dc_chat_t) -> libc::c_i eprintln!("ignoring careless call to dc_chat_get_archived()"); return 0; } - - let chat = &*chat; - - chat.is_archived() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.is_archived() as libc::c_int } #[no_mangle] @@ -1827,10 +1826,8 @@ pub unsafe extern "C" fn dc_chat_is_unpromoted(chat: *mut dc_chat_t) -> libc::c_ eprintln!("ignoring careless call to dc_chat_is_unpromoted()"); return 0; } - - let chat = &*chat; - - chat.is_unpromoted() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.is_unpromoted() as libc::c_int } #[no_mangle] @@ -1839,10 +1836,8 @@ pub unsafe extern "C" fn dc_chat_is_self_talk(chat: *mut dc_chat_t) -> libc::c_i eprintln!("ignoring careless call to dc_chat_is_self_talk()"); return 0; } - - let chat = &*chat; - - chat.is_self_talk() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.is_self_talk() as libc::c_int } #[no_mangle] @@ -1851,10 +1846,8 @@ pub unsafe extern "C" fn dc_chat_is_verified(chat: *mut dc_chat_t) -> libc::c_in eprintln!("ignoring careless call to dc_chat_is_verified()"); return 0; } - - let chat = &*chat; - - chat.is_verified() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.is_verified() as libc::c_int } #[no_mangle] @@ -1863,10 +1856,8 @@ pub unsafe extern "C" fn dc_chat_is_sending_locations(chat: *mut dc_chat_t) -> l eprintln!("ignoring careless call to dc_chat_is_sending_locations()"); return 0; } - - let chat = &*chat; - - chat.is_sending_locations() as libc::c_int + let ffi_chat = &*chat; + ffi_chat.chat.is_sending_locations() as libc::c_int } // dc_msg_t @@ -2095,9 +2086,14 @@ pub unsafe extern "C" fn dc_msg_get_summary( eprintln!("ignoring careless call to dc_msg_get_summary()"); return ptr::null_mut(); } - let chat = if chat.is_null() { None } else { Some(&*chat) }; + let maybe_chat = if chat.is_null() { + None + } else { + let ffi_chat = &*chat; + Some(&ffi_chat.chat) + }; let ffi_msg = &mut *msg; - let lot = message::dc_msg_get_summary(&*ffi_msg.context, &mut ffi_msg.message, chat); + let lot = message::dc_msg_get_summary(&*ffi_msg.context, &mut ffi_msg.message, maybe_chat); Box::into_raw(Box::new(lot)) } diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index f8cfbfe24..5e54343c1 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -581,7 +581,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E for i in (0..cnt).rev() { let chat = Chat::load_from_db(context, chatlist.get_chat_id(i))?; - let temp_subtitle = chat.get_subtitle(); + let temp_subtitle = chat.get_subtitle(context); let temp_name = chat.get_name(); info!( context, @@ -647,7 +647,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E let sel_chat = sel_chat.as_ref().unwrap(); let msglist = chat::get_chat_msgs(context, sel_chat.get_id(), 0x1, 0); - let temp2 = sel_chat.get_subtitle(); + let temp2 = sel_chat.get_subtitle(context); let temp_name = sel_chat.get_name(); info!( context, diff --git a/src/chat.rs b/src/chat.rs index 0fbabcf70..277990652 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -21,8 +21,7 @@ use std::ptr; /// and are not updated on database changes; /// if you want an update, you have to recreate the object. #[derive(Clone)] -pub struct Chat<'a> { - pub context: &'a Context, +pub struct Chat { pub id: u32, pub typ: Chattype, pub name: String, @@ -34,8 +33,8 @@ pub struct Chat<'a> { is_sending_locations: bool, } -impl<'a> Chat<'a> { - pub fn load_from_db(context: &'a Context, chat_id: u32) -> Result { +impl Chat { + pub fn load_from_db(context: &Context, chat_id: u32) -> Result { let res = context.sql.query_row( "SELECT c.id,c.type,c.name, c.grpid,c.param,c.archived, \ c.blocked, c.gossiped_timestamp, c.locations_send_until \ @@ -43,7 +42,6 @@ impl<'a> Chat<'a> { params![chat_id as i32], |row| { let c = Chat { - context, id: row.get(0)?, typ: row.get(1)?, name: row.get::<_, String>(2)?, @@ -73,23 +71,23 @@ impl<'a> Chat<'a> { Ok(mut chat) => { match chat.id { DC_CHAT_ID_DEADDROP => { - chat.name = chat.context.stock_str(StockMessage::DeadDrop).into(); + chat.name = context.stock_str(StockMessage::DeadDrop).into(); } DC_CHAT_ID_ARCHIVED_LINK => { - let tempname = chat.context.stock_str(StockMessage::ArchivedChats); - let cnt = dc_get_archived_cnt(chat.context); + let tempname = context.stock_str(StockMessage::ArchivedChats); + let cnt = dc_get_archived_cnt(context); chat.name = format!("{} ({})", tempname, cnt); } DC_CHAT_ID_STARRED => { - chat.name = chat.context.stock_str(StockMessage::StarredMsgs).into(); + chat.name = context.stock_str(StockMessage::StarredMsgs).into(); } _ => { if chat.typ == Chattype::Single { - let contacts = get_chat_contacts(chat.context, chat.id); + let contacts = get_chat_contacts(context, chat.id); let mut chat_name = "Err [Name not found]".to_owned(); if !(*contacts).is_empty() { - if let Ok(contact) = Contact::get_by_id(chat.context, contacts[0]) { + if let Ok(contact) = Contact::get_by_id(context, contacts[0]) { chat_name = contact.get_display_name().to_owned(); } } @@ -98,7 +96,7 @@ impl<'a> Chat<'a> { } if chat.param.exists(Param::Selftalk) { - chat.name = chat.context.stock_str(StockMessage::SelfMsg).into(); + chat.name = context.stock_str(StockMessage::SelfMsg).into(); } } } @@ -111,10 +109,10 @@ impl<'a> Chat<'a> { self.param.exists(Param::Selftalk) } - pub fn update_param(&mut self) -> Result<(), Error> { + pub fn update_param(&mut self, context: &Context) -> Result<(), Error> { sql::execute( - self.context, - &self.context.sql, + context, + &context.sql, "UPDATE chats SET param=? WHERE id=?", params![self.param.to_string(), self.id as i32], ) @@ -132,22 +130,18 @@ impl<'a> Chat<'a> { &self.name } - pub fn get_subtitle(&self) -> String { + pub fn get_subtitle(&self, context: &Context) -> String { // returns either the address or the number of chat members if self.typ == Chattype::Single && self.param.exists(Param::Selftalk) { - return self - .context - .stock_str(StockMessage::SelfTalkSubTitle) - .into(); + return context.stock_str(StockMessage::SelfTalkSubTitle).into(); } if self.typ == Chattype::Single { - return self - .context + return context .sql .query_row_col( - self.context, + context, "SELECT c.addr FROM chats_contacts cc \ LEFT JOIN contacts c ON c.id=cc.contact_id \ WHERE cc.chat_id=?;", @@ -159,11 +153,10 @@ impl<'a> Chat<'a> { if self.typ == Chattype::Group || self.typ == Chattype::VerifiedGroup { if self.id == 1 { - return self.context.stock_str(StockMessage::DeadDrop).into(); + return context.stock_str(StockMessage::DeadDrop).into(); } - let cnt = get_chat_contact_cnt(self.context, self.id); - return self - .context + let cnt = get_chat_contact_cnt(context, self.id); + return context .stock_string_repl_int(StockMessage::Member, cnt) .into(); } @@ -171,10 +164,10 @@ impl<'a> Chat<'a> { return "Err".into(); } - pub fn get_parent_mime_headers(&self) -> Option<(String, String, String)> { + pub fn get_parent_mime_headers(&self, context: &Context) -> Option<(String, String, String)> { let collect = |row: &rusqlite::Row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)); let params = params![self.id as i32, DC_CONTACT_ID_SELF as i32]; - let sql = &self.context.sql; + let sql = &context.sql; let main_query = "SELECT rfc724_mid, mime_in_reply_to, mime_references \ FROM msgs WHERE chat_id=?1 AND timestamp=(SELECT max(timestamp) \ FROM msgs WHERE chat_id=?1 AND from_id!=?2);"; @@ -187,15 +180,15 @@ impl<'a> Chat<'a> { .ok() } - pub fn get_profile_image(&self) -> Option { + pub fn get_profile_image(&self, context: &Context) -> Option { if let Some(image_rel) = self.param.get(Param::ProfileImage) { if !image_rel.is_empty() { - return Some(dc_get_abs_path_safe(self.context, image_rel)); + return Some(dc_get_abs_path_safe(context, image_rel)); } } else if self.typ == Chattype::Single { - let contacts = get_chat_contacts(self.context, self.id); + let contacts = get_chat_contacts(context, self.id); if !contacts.is_empty() { - if let Ok(contact) = Contact::get_by_id(self.context, contacts[0]) { + if let Ok(contact) = Contact::get_by_id(context, contacts[0]) { return contact.get_profile_image(); } } @@ -204,13 +197,13 @@ impl<'a> Chat<'a> { None } - pub fn get_color(&self) -> u32 { + pub fn get_color(&self, context: &Context) -> u32 { let mut color = 0; if self.typ == Chattype::Single { - let contacts = get_chat_contacts(self.context, self.id); + let contacts = get_chat_contacts(context, self.id); if !contacts.is_empty() { - if let Ok(contact) = Contact::get_by_id(self.context, contacts[0]) { + if let Ok(contact) = Contact::get_by_id(context, contacts[0]) { color = contact.get_color(); } } @@ -304,7 +297,7 @@ impl<'a> Chat<'a> { if self.typ == Chattype::Group || self.typ == Chattype::VerifiedGroup { if self.param.get_int(Param::Unpromoted).unwrap_or_default() == 1 { self.param.remove(Param::Unpromoted); - self.update_param().unwrap(); + self.update_param(context).unwrap(); } } } @@ -377,7 +370,7 @@ impl<'a> Chat<'a> { msg.param.remove(Param::ErroneousE2ee); if !self.is_self_talk() { if let Some((parent_rfc724_mid, parent_in_reply_to, parent_references)) = - self.get_parent_mime_headers() + self.get_parent_mime_headers(context) { if !parent_rfc724_mid.is_empty() { new_in_reply_to = parent_rfc724_mid.clone(); @@ -1354,7 +1347,7 @@ pub fn add_contact_to_chat_ex( && chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 1 { chat.param.remove(Param::Unpromoted); - chat.update_param().unwrap(); + chat.update_param(context).unwrap(); } let self_addr = context .sql @@ -1673,7 +1666,7 @@ pub fn set_chat_profile_image( } chat.param.set(Param::ProfileImage, &new_image_rel); - if chat.update_param().is_ok() { + if chat.update_param(context).is_ok() { if chat.is_promoted() { let mut msg = dc_msg_new_untyped(); msg.param.set_int(Param::Cmd, DC_CMD_GROUPIMAGE_CHANGED); diff --git a/src/chatlist.rs b/src/chatlist.rs index 72d6ca566..03e959069 100644 --- a/src/chatlist.rs +++ b/src/chatlist.rs @@ -247,7 +247,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<'a>>) -> Lot { + pub fn get_summary(&self, 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". diff --git a/src/dc_mimefactory.rs b/src/dc_mimefactory.rs index 23143375f..0c745f65c 100644 --- a/src/dc_mimefactory.rs +++ b/src/dc_mimefactory.rs @@ -38,7 +38,7 @@ pub struct dc_mimefactory_t<'a> { pub rfc724_mid: *mut libc::c_char, pub loaded: dc_mimefactory_loaded_t, pub msg: Message, - pub chat: Option>, + pub chat: Option, pub increation: libc::c_int, pub in_reply_to: *mut libc::c_char, pub references: *mut libc::c_char, @@ -995,7 +995,8 @@ pub unsafe fn dc_mimefactory_render( e.as_ptr(), ); } else { - subject_str = get_subject(factory.chat.as_ref(), &mut factory.msg, afwd_email) + subject_str = + get_subject(context, factory.chat.as_ref(), &mut factory.msg, afwd_email) } subject = mailimf_subject_new(dc_encode_header_words(subject_str)); mailimf_fields_add( @@ -1061,6 +1062,7 @@ pub unsafe fn dc_mimefactory_render( } unsafe fn get_subject( + context: &Context, chat: Option<&Chat>, msg: &mut Message, afwd_email: libc::c_int, @@ -1070,7 +1072,6 @@ unsafe fn get_subject( } let chat = chat.unwrap(); - let context = chat.context; let ret: *mut libc::c_char; let raw_subject = { diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 805a4c2db..a9aa67402 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -1354,7 +1354,7 @@ unsafe fn create_or_lookup_group( } else { chat.param.set(Param::ProfileImage, grpimage); } - chat.update_param().unwrap(); + chat.update_param(context).unwrap(); send_EVENT_CHAT_MODIFIED = 1; } } diff --git a/src/message.rs b/src/message.rs index bdc86d2fe..a4432407c 100644 --- a/src/message.rs +++ b/src/message.rs @@ -744,7 +744,7 @@ pub fn dc_msg_get_summary(context: &Context, msg: &mut Message, chat: Option<&Ch let contact = if msg.from_id != DC_CONTACT_ID_SELF as libc::c_uint && ((*chat).typ == Chattype::Group || (*chat).typ == Chattype::VerifiedGroup) { - Contact::get_by_id((*chat).context, msg.from_id).ok() + Contact::get_by_id(context, msg.from_id).ok() } else { None };