refactor(chat): store rust strings in the chat struct

deltachat-ffi/src/lib.rs

@@ -654,7 +654,7 @@ pub unsafe extern "C" fn dc_set_chat_name(
     assert!(chat_id > constants::DC_CHAT_ID_LAST_SPECIAL as u32);
     let context = &*context;
 
-    chat::set_chat_name(context, chat_id, name)
+    chat::set_chat_name(context, chat_id, as_str(name))
 }
 
 #[no_mangle]
@@ -1309,7 +1309,7 @@ pub unsafe extern "C" fn dc_chat_get_name(chat: *mut dc_chat_t) -> *mut libc::c_
     assert!(!chat.is_null());
     let chat = &*chat;
 
-    chat.get_name()
+    chat.get_name().strdup()
 }
 
 #[no_mangle]
@@ -1317,7 +1317,7 @@ pub unsafe extern "C" fn dc_chat_get_subtitle(chat: *mut dc_chat_t) -> *mut libc
     assert!(!chat.is_null());
     let chat = &*chat;
 
-    chat.get_subtitle()
+    chat.get_subtitle().strdup()
 }
 
 #[no_mangle]

examples/repl/cmdline.rs

@@ -615,12 +615,10 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
                         "{}#{}: {} [{}] [{} fresh]",
                         chat_prefix(&chat),
                         chat.get_id(),
-                        as_str(temp_name),
+                        temp_name,
-                        as_str(temp_subtitle),
+                        temp_subtitle,
                         chat::get_fresh_msg_cnt(context, chat.get_id()),
                     );
-                    free(temp_subtitle as *mut libc::c_void);
-                    free(temp_name as *mut libc::c_void);
                     let lot = chatlist.get_summary(i, Some(&chat));
                     let statestr = if chat.is_archived() {
                         " [Archived]"
@@ -688,16 +686,14 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
                 "{}#{}: {} [{}]{}",
                 chat_prefix(sel_chat),
                 sel_chat.get_id(),
-                as_str(temp_name),
+                temp_name,
-                as_str(temp2),
+                temp2,
                 if sel_chat.is_sending_locations() {
                     "📍"
                 } else {
                     ""
                 },
             );
-            free(temp_name as *mut libc::c_void);
-            free(temp2 as *mut libc::c_void);
             if !msglist.is_null() {
                 log_msglist(context, msglist);
                 dc_array_unref(msglist);
@@ -772,7 +768,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
         "groupname" => {
             ensure!(sel_chat.is_some(), "No chat selected.");
             ensure!(!arg1.is_empty(), "Argument <name> missing.");
-            if 0 != chat::set_chat_name(context, sel_chat.as_ref().unwrap().get_id(), arg1_c) {
+            if 0 != chat::set_chat_name(context, sel_chat.as_ref().unwrap().get_id(), arg1) {
                 println!("Chat name set");
             } else {
                 bail!("Failed to set chat name");

src/chat.rs

@@ -26,9 +26,9 @@ pub struct Chat<'a> {
     pub context: &'a Context,
     pub id: u32,
     pub typ: Chattype,
-    pub name: *mut libc::c_char,
+    pub name: String,
     archived: bool,
-    pub grpid: *mut libc::c_char,
+    pub grpid: String,
     blocked: Blocked,
     pub param: Params,
     pub gossiped_timestamp: i64,
@@ -43,30 +43,19 @@ impl<'a> Chat<'a> {
              FROM chats c WHERE c.id=?;",
             params![chat_id as i32],
             |row| {
-                let mut c = Chat {
+                let c = Chat {
                     context,
-                    id: 0,
+                    id: row.get(0)?,
-                    typ: Chattype::Undefined,
+                    typ: row.get(1)?,
-                    name: std::ptr::null_mut(),
+                    name: row.get::<_, String>(2)?,
-                    archived: false,
+                    grpid: row.get::<_, String>(3)?,
-                    grpid: std::ptr::null_mut(),
+                    param: row.get::<_, String>(4)?.parse().unwrap_or_default(),
-                    blocked: Blocked::Not,
+                    archived: row.get(5)?,
-                    param: Params::new(),
+                    blocked: row.get::<_, Option<_>>(6)?.unwrap_or_default(),
-                    gossiped_timestamp: 0,
+                    gossiped_timestamp: row.get(7)?,
-                    is_sending_locations: false,
+                    is_sending_locations: row.get(8)?,
                 };
 
-                c.id = row.get(0)?;
-                c.typ = row.get(1)?;
-                c.name = unsafe { row.get::<_, String>(2)?.strdup() };
-                c.grpid = unsafe { row.get::<_, String>(3)?.strdup() };
-
-                c.param = row.get::<_, String>(4)?.parse().unwrap_or_default();
-                c.archived = row.get(5)?;
-                c.blocked = row.get::<_, Option<_>>(6)?.unwrap_or_default();
-                c.gossiped_timestamp = row.get(7)?;
-                c.is_sending_locations = row.get(8)?;
-
                 Ok(c)
             },
         );
@@ -84,44 +73,33 @@ impl<'a> Chat<'a> {
             },
             Ok(mut chat) => {
                 match chat.id {
-                    1 => unsafe {
+                    1 => {
-                        free(chat.name.cast());
+                        chat.name = chat.context.stock_str(StockMessage::DeadDrop).into();
-                        chat.name = chat.context.stock_str(StockMessage::DeadDrop).strdup();
+                    }
-                    },
+                    6 => {
-                    6 => unsafe {
-                        free(chat.name.cast());
                         let tempname = chat.context.stock_str(StockMessage::ArchivedChats);
                         let cnt = dc_get_archived_cnt(chat.context);
-                        chat.name = format!("{} ({})", tempname, cnt).strdup();
+                        chat.name = format!("{} ({})", tempname, cnt);
-                    },
+                    }
-                    5 => unsafe {
+                    5 => {
-                        free(chat.name.cast());
+                        chat.name = chat.context.stock_str(StockMessage::StarredMsgs).into();
-                        chat.name = chat.context.stock_str(StockMessage::StarredMsgs).strdup();
+                    }
-                    },
                     _ => {
-                        unsafe {
                         if chat.typ == Chattype::Single {
-                                free(chat.name.cast());
                             let contacts = get_chat_contacts(chat.context, chat.id);
                             let mut chat_name = "Err [Name not found]".to_owned();
 
                             if !(*contacts).is_empty() {
-                                    if let Ok(contact) =
+                                if let Ok(contact) = Contact::get_by_id(chat.context, contacts[0]) {
-                                        Contact::get_by_id(chat.context, contacts[0])
-                                    {
                                     chat_name = contact.get_display_name().to_owned();
                                 }
                             }
 
-                                chat.name = (&chat_name).strdup();
+                            chat.name = chat_name;
-                            }
                         }
 
                         if chat.param.exists(Param::Selftalk) {
-                            unsafe {
+                            chat.name = chat.context.stock_str(StockMessage::SelfMsg).into();
-                                free(chat.name as *mut libc::c_void);
-                                chat.name = chat.context.stock_str(StockMessage::SelfMsg).strdup();
-                            }
                         }
                     }
                 }
@@ -143,29 +121,30 @@ impl<'a> Chat<'a> {
         )
     }
 
-    pub unsafe fn get_id(&self) -> u32 {
+    pub fn get_id(&self) -> u32 {
         self.id
     }
 
-    pub unsafe fn get_type(&self) -> Chattype {
+    pub fn get_type(&self) -> Chattype {
         self.typ
     }
 
-    pub unsafe fn get_name(&self) -> *mut libc::c_char {
+    pub fn get_name(&self) -> &str {
-        dc_strdup(self.name)
+        &self.name
     }
 
-    pub unsafe fn get_subtitle(&self) -> *mut libc::c_char {
+    pub fn get_subtitle(&self) -> String {
-        /* returns either the address or the number of chat members */
+        // returns either the address or the number of chat members
 
-        let mut ret: *mut libc::c_char = std::ptr::null_mut();
         if self.typ == Chattype::Single && self.param.exists(Param::Selftalk) {
-            ret = self
+            return self
                 .context
                 .stock_str(StockMessage::SelfTalkSubTitle)
-                .strdup();
+                .into();
-        } else if self.typ == Chattype::Single {
+        }
-            let ret_raw: String = self
+
+        if self.typ == Chattype::Single {
+            return self
                 .context
                 .sql
                 .query_row_col(
@@ -177,24 +156,22 @@ impl<'a> Chat<'a> {
                     0,
                 )
                 .unwrap_or_else(|| "Err".into());
-            ret = ret_raw.strdup();
+        }
-        } else if self.typ == Chattype::Group || self.typ == Chattype::VerifiedGroup {
+
+        if self.typ == Chattype::Group || self.typ == Chattype::VerifiedGroup {
             if self.id == 1 {
-                ret = self.context.stock_str(StockMessage::DeadDrop).strdup();
+                return self.context.stock_str(StockMessage::DeadDrop).into();
-            } else {
+            }
             let cnt = get_chat_contact_cnt(self.context, self.id);
-                ret = self
+            return self
                 .context
                 .stock_string_repl_int(StockMessage::Member, cnt)
-                    .strdup();
+                .into();
-            }
-        }
-        if !ret.is_null() {
-            ret
-        } else {
-            dc_strdup(b"Err\x00" as *const u8 as *const libc::c_char)
         }
+
+        return "Err".into();
     }
+
     unsafe fn get_parent_mime_headers(
         &self,
         parent_rfc724_mid: *mut *mut libc::c_char,
@@ -264,7 +241,7 @@ impl<'a> Chat<'a> {
     }
 
     pub fn get_color(&self) -> u32 {
-        let mut color: u32 = 0i32 as u32;
+        let mut color = 0;
 
         if self.typ == Chattype::Single {
             let contacts = get_chat_contacts(self.context, self.id);
@@ -274,7 +251,7 @@ impl<'a> Chat<'a> {
                 }
             }
         } else {
-            color = unsafe { dc_str_to_color(self.name) } as u32
+            color = dc_str_to_color(&self.name);
         }
 
         color
@@ -338,7 +315,7 @@ impl<'a> Chat<'a> {
                 let from_c = CString::yolo(from.unwrap());
                 new_rfc724_mid = dc_create_outgoing_rfc724_mid(
                     if self.typ == Chattype::Group || self.typ == Chattype::VerifiedGroup {
-                        self.grpid
+                        self.grpid.strdup()
                     } else {
                         ptr::null_mut()
                     },
@@ -1663,7 +1640,7 @@ pub unsafe fn remove_contact_from_chat(
                 if chat.param.get_int(Param::Unpromoted).unwrap_or_default() == 0 {
                     (*msg).type_0 = Viewtype::Text;
                     if contact.id == DC_CONTACT_ID_SELF as u32 {
-                        set_group_explicitly_left(context, chat.grpid);
+                        set_group_explicitly_left(context, chat.grpid).unwrap();
                         (*msg).text = Some(context.stock_system_msg(
                             StockMessage::MsgGroupLeft,
                             "",
@@ -1707,41 +1684,38 @@ pub unsafe fn remove_contact_from_chat(
     success
 }
 
-// Should return Result
+fn set_group_explicitly_left(context: &Context, grpid: impl AsRef<str>) -> Result<(), Error> {
-fn set_group_explicitly_left(context: &Context, grpid: *const libc::c_char) {
+    if !is_group_explicitly_left(context, grpid.as_ref())? {
-    if 0 == is_group_explicitly_left(context, grpid) {
         sql::execute(
             context,
             &context.sql,
             "INSERT INTO leftgrps (grpid) VALUES(?);",
-            params![as_str(grpid)],
+            params![grpid.as_ref()],
-        )
+        )?;
-        .ok();
     }
+
+    Ok(())
 }
 
 // TODO should return bool /rtn
-pub fn is_group_explicitly_left(context: &Context, grpid: *const libc::c_char) -> libc::c_int {
+pub fn is_group_explicitly_left(context: &Context, grpid: impl AsRef<str>) -> Result<bool, Error> {
-    context
+    context.sql.exists(
-        .sql
-        .exists(
         "SELECT id FROM leftgrps WHERE grpid=?;",
-            params![as_str(grpid)],
+        params![grpid.as_ref()],
     )
-        .unwrap_or_default() as libc::c_int
 }
 
 // TODO should return bool /rtn
 pub unsafe fn set_chat_name(
     context: &Context,
     chat_id: u32,
-    new_name: *const libc::c_char,
+    new_name: impl AsRef<str>,
 ) -> libc::c_int {
     /* the function only sets the names of group chats; normal chats get their names from the contacts */
     let mut success: libc::c_int = 0i32;
     let mut msg = dc_msg_new_untyped(context);
 
-    if new_name.is_null() || *new_name.offset(0isize) as libc::c_int == 0 || chat_id <= 9 {
+    if new_name.as_ref().is_empty() || chat_id <= DC_CHAT_ID_LAST_SPECIAL as u32 {
         return 0;
     }
 
@@ -1749,9 +1723,9 @@ pub unsafe fn set_chat_name(
 
     if !(0i32 == real_group_exists(context, chat_id) || chat.is_err()) {
         let chat = chat.unwrap();
-        if strcmp(chat.name, new_name) == 0i32 {
+        if &chat.name == new_name.as_ref() {
-            success = 1i32
+            success = 1;
-        } else if !(is_contact_in_chat(context, chat_id, 1i32 as u32) == 1i32) {
+        } else if !(is_contact_in_chat(context, chat_id, 1) == 1) {
             log_event!(
                 context,
                 Event::ERROR_SELF_NOT_IN_GROUP,
@@ -1765,7 +1739,7 @@ pub unsafe fn set_chat_name(
                 &context.sql,
                 format!(
                     "UPDATE chats SET name='{}' WHERE id={};",
-                    as_str(new_name),
+                    new_name.as_ref(),
                     chat_id as i32
                 ),
                 params![],
@@ -1776,13 +1750,13 @@ pub unsafe fn set_chat_name(
                     (*msg).type_0 = Viewtype::Text;
                     (*msg).text = Some(context.stock_system_msg(
                         StockMessage::MsgGrpName,
-                        as_str(chat.name),
+                        &chat.name,
-                        as_str(new_name),
+                        new_name.as_ref(),
                         DC_CONTACT_ID_SELF as u32,
                     ));
                     (*msg).param.set_int(Param::Cmd, 2);
-                    if !chat.name.is_null() {
+                    if !chat.name.is_empty() {
-                        (*msg).param.set(Param::Arg, as_str(chat.name));
+                        (*msg).param.set(Param::Arg, &chat.name);
                     }
                     (*msg).id = send_msg(context, chat_id, msg).unwrap_or_default();
                     context.call_cb(
@@ -2088,12 +2062,3 @@ pub fn add_device_msg(context: &Context, chat_id: u32, text: *const libc::c_char
         msg_id as uintptr_t,
     );
 }
-
-impl<'a> Drop for Chat<'a> {
-    fn drop(&mut self) {
-        unsafe {
-            free(self.name.cast());
-            free(self.grpid.cast());
-        }
-    }
-}

src/contact.rs

@@ -791,7 +791,7 @@ impl<'a> Contact<'a> {
     /// and can be used for an fallback avatar with white initials
     /// as well as for headlines in bubbles of group chats.
     pub fn get_color(&self) -> u32 {
-        dc_str_to_color_safe(&self.addr)
+        dc_str_to_color(&self.addr)
     }
 
     /// Check if a contact was verified. E.g. by a secure-join QR code scan

src/dc_mimefactory.rs

@@ -548,14 +548,15 @@ pub unsafe fn dc_mimefactory_render(mut factory: *mut dc_mimefactory_t) -> libc:
                     imf_fields,
                     mailimf_field_new_custom(
                         strdup(b"Chat-Group-ID\x00" as *const u8 as *const libc::c_char),
-                        dc_strdup(chat.grpid),
+                        chat.grpid.strdup(),
                     ),
                 );
+                let name = CString::yolo(chat.name.as_bytes());
                 mailimf_fields_add(
                     imf_fields,
                     mailimf_field_new_custom(
                         strdup(b"Chat-Group-Name\x00" as *const u8 as *const libc::c_char),
-                        dc_encode_header_words(chat.name),
+                        dc_encode_header_words(name.as_ptr()),
                     ),
                 );
                 if command == 5 {
@@ -1090,12 +1091,13 @@ unsafe fn get_subject(
     if (*msg).param.get_int(Param::Cmd).unwrap_or_default() == 6 {
         ret = context.stock_str(StockMessage::AcSetupMsgSubject).strdup()
     } else if chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup {
-        ret = dc_mprintf(
+        ret = format!(
-            b"Chat: %s: %s%s\x00" as *const u8 as *const libc::c_char,
+            "Chat: {}: {}{}",
             chat.name,
-            fwd,
+            to_string(fwd),
-            raw_subject,
+            to_string(raw_subject),
         )
+        .strdup()
     } else {
         ret = dc_mprintf(
             b"Chat: %s%s\x00" as *const u8 as *const libc::c_char,

src/dc_receive_imf.rs

@@ -1041,7 +1041,7 @@ unsafe fn create_or_lookup_group(
     ret_chat_id: *mut uint32_t,
     ret_chat_id_blocked: &mut Blocked,
 ) {
-    let group_explicitly_left: libc::c_int;
+    let group_explicitly_left: bool;
     let mut chat_id = 0;
     let mut chat_id_blocked = Blocked::Not;
     let mut chat_id_verified = 0;
@@ -1252,7 +1252,8 @@ unsafe fn create_or_lookup_group(
     }
 
     // check if the group does not exist but should be created
-    group_explicitly_left = chat::is_group_explicitly_left(context, grpid);
+    group_explicitly_left =
+        chat::is_group_explicitly_left(context, as_str(grpid)).unwrap_or_default();
 
     let self_addr = context
         .sql
@@ -1265,7 +1266,7 @@ unsafe fn create_or_lookup_group(
             // otherwise, a pending "quit" message may pop up
             && X_MrRemoveFromGrp.is_null()
             // re-create explicitly left groups only if ourself is re-added
-            && (0 == group_explicitly_left
+            && (!group_explicitly_left
                 || !X_MrAddToGrp.is_null() && addr_cmp(&self_addr, as_str(X_MrAddToGrp)))
     {
         let mut create_verified: libc::c_int = 0;
@@ -1301,7 +1302,7 @@ unsafe fn create_or_lookup_group(
     // again, check chat_id
     if chat_id <= DC_CHAT_ID_LAST_SPECIAL as u32 {
         chat_id = 0;
-        if 0 != group_explicitly_left {
+        if group_explicitly_left {
             chat_id = DC_CHAT_ID_TRASH as u32;
         } else {
             create_or_lookup_adhoc_group(

src/dc_securejoin.rs

@@ -14,7 +14,6 @@ use crate::dc_lot::*;
 use crate::dc_mimeparser::*;
 use crate::dc_msg::*;
 use crate::dc_qr::*;
-use crate::dc_strencode::*;
 use crate::dc_token::*;
 use crate::dc_tools::*;
 use crate::key::*;
@@ -36,8 +35,6 @@ pub unsafe fn dc_get_securejoin_qr(
     let mut fingerprint = 0 as *mut libc::c_char;
     let mut invitenumber: *mut libc::c_char;
     let mut auth: *mut libc::c_char;
-    let mut group_name = 0 as *mut libc::c_char;
-    let mut group_name_urlencoded = 0 as *mut libc::c_char;
     let mut qr: Option<String> = None;
 
     dc_ensure_secret_key_exists(context).ok();
@@ -53,12 +50,10 @@ pub unsafe fn dc_get_securejoin_qr(
     }
     let self_addr = context.sql.get_config(context, "configured_addr");
 
-    let cleanup = |fingerprint, group_name, group_name_urlencoded| {
+    let cleanup = |fingerprint| {
         free(fingerprint as *mut libc::c_void);
         free(invitenumber as *mut libc::c_void);
         free(auth as *mut libc::c_void);
-        free(group_name as *mut libc::c_void);
-        free(group_name_urlencoded as *mut libc::c_void);
 
         if let Some(qr) = qr {
             qr.strdup()
@@ -69,7 +64,7 @@ pub unsafe fn dc_get_securejoin_qr(
 
     if self_addr.is_none() {
         error!(context, 0, "Not configured, cannot generate QR code.",);
-        return cleanup(fingerprint, group_name, group_name_urlencoded);
+        return cleanup(fingerprint);
     }
 
     let self_addr = self_addr.unwrap();
@@ -81,7 +76,7 @@ pub unsafe fn dc_get_securejoin_qr(
     fingerprint = get_self_fingerprint(context);
 
     if fingerprint.is_null() {
-        return cleanup(fingerprint, group_name, group_name_urlencoded);
+        return cleanup(fingerprint);
     }
 
     let self_addr_urlencoded = utf8_percent_encode(&self_addr, NON_ALPHANUMERIC).to_string();
@@ -89,15 +84,16 @@ pub unsafe fn dc_get_securejoin_qr(
 
     qr = if 0 != group_chat_id {
         if let Ok(chat) = Chat::load_from_db(context, group_chat_id) {
-            group_name = chat.get_name();
+            let group_name = chat.get_name();
-            group_name_urlencoded = dc_urlencode(group_name);
+            let group_name_urlencoded =
+                utf8_percent_encode(&group_name, NON_ALPHANUMERIC).to_string();
 
             Some(format!(
                 "OPENPGP4FPR:{}#a={}&g={}&x={}&i={}&s={}",
                 as_str(fingerprint),
                 self_addr_urlencoded,
-                as_str(group_name_urlencoded),
+                &group_name_urlencoded,
-                as_str(chat.grpid),
+                &chat.grpid,
                 as_str(invitenumber),
                 as_str(auth),
             ))
@@ -106,7 +102,7 @@ pub unsafe fn dc_get_securejoin_qr(
                 context,
                 0, "Cannot get QR-code for chat-id {}", group_chat_id,
             );
-            return cleanup(fingerprint, group_name, group_name_urlencoded);
+            return cleanup(fingerprint);
         }
     } else {
         Some(format!(
@@ -121,7 +117,7 @@ pub unsafe fn dc_get_securejoin_qr(
 
     info!(context, 0, "Generated QR code: {}", qr.as_ref().unwrap());
 
-    cleanup(fingerprint, group_name, group_name_urlencoded)
+    cleanup(fingerprint)
 }
 
 fn get_self_fingerprint(context: &Context) -> *mut libc::c_char {

src/dc_tools.rs

@@ -532,7 +532,7 @@ const COLORS: [u32; 16] = [
     0x39b249, 0xbb243b, 0x964078, 0x66874f, 0x308ab9, 0x127ed0, 0xbe450c,
 ];
 
-pub fn dc_str_to_color_safe(s: impl AsRef<str>) -> u32 {
+pub fn dc_str_to_color(s: impl AsRef<str>) -> u32 {
     let str_lower = s.as_ref().to_lowercase();
     let mut checksum = 0;
     let bytes = str_lower.as_bytes();
@@ -545,10 +545,6 @@ pub fn dc_str_to_color_safe(s: impl AsRef<str>) -> u32 {
     COLORS[color_index]
 }
 
-pub unsafe fn dc_str_to_color(str: *const libc::c_char) -> libc::c_int {
-    dc_str_to_color_safe(as_str(str)) as libc::c_int
-}
-
 /* clist tools */
 /* calls free() for each item content */
 pub unsafe fn clist_free_content(haystack: *const clist) {

tests/stress.rs

@@ -767,7 +767,7 @@ fn test_chat() {
         assert_eq!(chat2_id, chat_id);
         let chat2 = Chat::load_from_db(&context.ctx, chat2_id).unwrap();
 
-        assert_eq!(as_str(chat2.name), as_str(chat.name));
+        assert_eq!(chat2.name, chat.name);
     }
 }