refactor: unsafe, CStr and libc moved out

Cargo.lock

@@ -686,6 +686,7 @@ version = "1.0.0-beta.8"
 dependencies = [
  "deltachat 1.0.0-beta.8",
  "deltachat-provider-database 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "failure 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "human-panic 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.66 (registry+https://github.com/rust-lang/crates.io-index)",
  "num-traits 0.2.10 (registry+https://github.com/rust-lang/crates.io-index)",

deltachat-ffi/Cargo.toml

@@ -20,6 +20,7 @@ deltachat-provider-database = "0.2.1"
 libc = "0.2"
 human-panic = "1.0.1"
 num-traits = "0.2.6"
+failure = "0.1.6"
 
 [features]
 default = ["vendored", "nightly", "ringbuf"]

deltachat-ffi/src/lib.rs

@@ -25,13 +25,13 @@ use num_traits::{FromPrimitive, ToPrimitive};
 use deltachat::constants::DC_MSG_ID_LAST_SPECIAL;
 use deltachat::contact::Contact;
 use deltachat::context::Context;
-use deltachat::dc_tools::{
-    as_path, dc_strdup, to_opt_string_lossy, to_string_lossy, OsStrExt, StrExt,
-};
 use deltachat::message::MsgId;
 use deltachat::stock::StockMessage;
 use deltachat::*;
 
+mod string;
+use self::string::*;
+
 // as C lacks a good and portable error handling,
 // in general, the C Interface is forgiving wrt to bad parameters.
 // - objects returned by some functions

deltachat-ffi/src/providers.rs

@@ -2,7 +2,7 @@ extern crate deltachat_provider_database;
 
 use std::ptr;
 
-use deltachat::dc_tools::{to_string_lossy, StrExt};
+use crate::string::{to_string_lossy, StrExt};
 use deltachat_provider_database::StatusState;
 
 #[no_mangle]

deltachat-ffi/src/string.rs

@@ -0,0 +1,350 @@
+use failure::Fail;
+use std::ffi::{CStr, CString};
+
+/// Duplicates a string
+///
+/// returns an empty string if NULL is given, never returns NULL (exits on errors)
+///
+/// # Examples
+///
+/// ```rust,norun
+/// use deltachat::dc_tools::{dc_strdup, to_string_lossy};
+/// unsafe {
+///     let str_a = b"foobar\x00" as *const u8 as *const libc::c_char;
+///     let str_a_copy = dc_strdup(str_a);
+///     assert_eq!(to_string_lossy(str_a_copy), "foobar");
+///     assert_ne!(str_a, str_a_copy);
+/// }
+/// ```
+pub unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char {
+    let ret: *mut libc::c_char;
+    if !s.is_null() {
+        ret = libc::strdup(s);
+        assert!(!ret.is_null());
+    } else {
+        ret = libc::calloc(1, 1) as *mut libc::c_char;
+        assert!(!ret.is_null());
+    }
+
+    ret
+}
+
+/// Error type for the [OsStrExt] trait
+#[derive(Debug, Fail, PartialEq)]
+pub enum CStringError {
+    /// The string contains an interior null byte
+    #[fail(display = "String contains an interior null byte")]
+    InteriorNullByte,
+    /// The string is not valid Unicode
+    #[fail(display = "String is not valid unicode")]
+    NotUnicode,
+}
+
+/// Extra convenience methods on [std::ffi::OsStr] to work with `*libc::c_char`.
+///
+/// The primary function of this trait is to more easily convert
+/// [OsStr], [OsString] or [Path] into pointers to C strings.  This always
+/// allocates a new string since it is very common for the source
+/// string not to have the required terminal null byte.
+///
+/// It is implemented for `AsRef<std::ffi::OsStr>>` trait, which
+/// allows any type which implements this trait to transparently use
+/// this.  This is how the conversion for [Path] works.
+///
+/// [OsStr]: std::ffi::OsStr
+/// [OsString]: std::ffi::OsString
+/// [Path]: std::path::Path
+///
+/// # Example
+///
+/// ```
+/// use deltachat::dc_tools::{dc_strdup, OsStrExt};
+/// let path = std::path::Path::new("/some/path");
+/// let path_c = path.to_c_string().unwrap();
+/// unsafe {
+///     let mut c_ptr: *mut libc::c_char = dc_strdup(path_c.as_ptr());
+/// }
+/// ```
+pub trait OsStrExt {
+    /// Convert a  [std::ffi::OsStr] to an [std::ffi::CString]
+    ///
+    /// This is useful to convert e.g. a [std::path::Path] to
+    /// [*libc::c_char] by using
+    /// [Path::as_os_str()](std::path::Path::as_os_str) and
+    /// [CStr::as_ptr()](std::ffi::CStr::as_ptr).
+    ///
+    /// This returns [CString] and not [&CStr] because not all [OsStr]
+    /// slices end with a null byte, particularly those coming from
+    /// [Path] do not have a null byte and having to handle this as
+    /// the caller would defeat the point of this function.
+    ///
+    /// On Windows this requires that the [OsStr] contains valid
+    /// unicode, which should normally be the case for a [Path].
+    ///
+    /// [CString]: std::ffi::CString
+    /// [CStr]: std::ffi::CStr
+    /// [OsStr]: std::ffi::OsStr
+    /// [Path]: std::path::Path
+    ///
+    /// # Errors
+    ///
+    /// Since a C `*char` is terminated by a NULL byte this conversion
+    /// will fail, when the [OsStr] has an interior null byte.  The
+    /// function will return
+    /// `[Err]([CStringError::InteriorNullByte])`.  When converting
+    /// from a [Path] it should be safe to
+    /// [`.unwrap()`](std::result::Result::unwrap) this anyway since a
+    /// [Path] should not contain interior null bytes.
+    ///
+    /// On windows when the string contains invalid Unicode
+    /// `[Err]([CStringError::NotUnicode])` is returned.
+    fn to_c_string(&self) -> Result<CString, CStringError>;
+}
+
+impl<T: AsRef<std::ffi::OsStr>> OsStrExt for T {
+    #[cfg(not(target_os = "windows"))]
+    fn to_c_string(&self) -> Result<CString, CStringError> {
+        use std::os::unix::ffi::OsStrExt;
+        CString::new(self.as_ref().as_bytes()).map_err(|err| match err {
+            std::ffi::NulError { .. } => CStringError::InteriorNullByte,
+        })
+    }
+
+    #[cfg(target_os = "windows")]
+    fn to_c_string(&self) -> Result<CString, CStringError> {
+        os_str_to_c_string_unicode(&self)
+    }
+}
+
+// Implementation for os_str_to_c_string on windows.
+#[allow(dead_code)]
+fn os_str_to_c_string_unicode(
+    os_str: &dyn AsRef<std::ffi::OsStr>,
+) -> Result<CString, CStringError> {
+    match os_str.as_ref().to_str() {
+        Some(val) => CString::new(val.as_bytes()).map_err(|err| match err {
+            std::ffi::NulError { .. } => CStringError::InteriorNullByte,
+        }),
+        None => Err(CStringError::NotUnicode),
+    }
+}
+
+/// Convenience methods/associated functions for working with [CString]
+///
+/// This is helps transitioning from unsafe code.
+pub trait CStringExt {
+    /// Create a new [CString], yolo style
+    ///
+    /// This unwrap the result, panicking when there are embedded NULL
+    /// bytes.
+    fn yolo<T: Into<Vec<u8>>>(t: T) -> CString {
+        CString::new(t).expect("String contains null byte, can not be CString")
+    }
+}
+
+impl CStringExt for CString {}
+
+/// Convenience methods to make transitioning from raw C strings easier.
+///
+/// To interact with (legacy) C APIs we often need to convert from
+/// Rust strings to raw C strings.  This can be clumsy to do correctly
+/// and the compiler sometimes allows it in an unsafe way.  These
+/// methods make it more succinct and help you get it right.
+pub trait StrExt {
+    /// Allocate a new raw C `*char` version of this string.
+    ///
+    /// This allocates a new raw C string which must be freed using
+    /// `free`.  It takes care of some common pitfalls with using
+    /// [CString.as_ptr].
+    ///
+    /// [CString.as_ptr]: std::ffi::CString.as_ptr
+    ///
+    /// # Panics
+    ///
+    /// This function will panic when the original string contains an
+    /// interior null byte as this can not be represented in raw C
+    /// strings.
+    unsafe fn strdup(&self) -> *mut libc::c_char;
+}
+
+impl<T: AsRef<str>> StrExt for T {
+    unsafe fn strdup(&self) -> *mut libc::c_char {
+        let tmp = CString::yolo(self.as_ref());
+        dc_strdup(tmp.as_ptr())
+    }
+}
+
+pub fn to_string_lossy(s: *const libc::c_char) -> String {
+    if s.is_null() {
+        return "".into();
+    }
+
+    let cstr = unsafe { CStr::from_ptr(s) };
+
+    cstr.to_string_lossy().to_string()
+}
+
+pub fn to_opt_string_lossy(s: *const libc::c_char) -> Option<String> {
+    if s.is_null() {
+        return None;
+    }
+
+    Some(to_string_lossy(s))
+}
+
+/// Convert a C `*char` pointer to a [std::path::Path] slice.
+///
+/// This converts a `*libc::c_char` pointer to a [Path] slice.  This
+/// essentially has to convert the pointer to [std::ffi::OsStr] to do
+/// so and thus is the inverse of [OsStrExt::to_c_string].  Just like
+/// [OsStrExt::to_c_string] requires valid Unicode on Windows, this
+/// requires that the pointer contains valid UTF-8 on Windows.
+///
+/// Because this returns a reference the [Path] silce can not outlive
+/// the original pointer.
+///
+/// [Path]: std::path::Path
+#[cfg(not(target_os = "windows"))]
+pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path {
+    assert!(!s.is_null(), "cannot be used on null pointers");
+    use std::os::unix::ffi::OsStrExt;
+    unsafe {
+        let c_str = std::ffi::CStr::from_ptr(s).to_bytes();
+        let os_str = std::ffi::OsStr::from_bytes(c_str);
+        std::path::Path::new(os_str)
+    }
+}
+
+// as_path() implementation for windows, documented above.
+#[cfg(target_os = "windows")]
+pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path {
+    as_path_unicode(s)
+}
+
+// Implementation for as_path() on Windows.
+//
+// Having this as a separate function means it can be tested on unix
+// too.
+#[allow(dead_code)]
+fn as_path_unicode<'a>(s: *const libc::c_char) -> &'a std::path::Path {
+    assert!(!s.is_null(), "cannot be used on null pointers");
+
+    let cstr = unsafe { CStr::from_ptr(s) };
+    let str = cstr.to_str().unwrap_or_else(|err| panic!("{}", err));
+
+    std::path::Path::new(str)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use libc::{free, strcmp};
+
+    #[test]
+    fn test_os_str_to_c_string_cwd() {
+        let some_dir = std::env::current_dir().unwrap();
+        some_dir.as_os_str().to_c_string().unwrap();
+    }
+
+    #[test]
+    fn test_os_str_to_c_string_unicode() {
+        let some_str = String::from("/some/valid/utf8");
+        let some_dir = std::path::Path::new(&some_str);
+        assert_eq!(
+            some_dir.as_os_str().to_c_string().unwrap(),
+            CString::new("/some/valid/utf8").unwrap()
+        );
+    }
+
+    #[test]
+    fn test_os_str_to_c_string_nul() {
+        let some_str = std::ffi::OsString::from("foo\x00bar");
+        assert_eq!(
+            some_str.to_c_string().err().unwrap(),
+            CStringError::InteriorNullByte
+        )
+    }
+
+    #[test]
+    fn test_path_to_c_string_cwd() {
+        let some_dir = std::env::current_dir().unwrap();
+        some_dir.to_c_string().unwrap();
+    }
+
+    #[test]
+    fn test_path_to_c_string_unicode() {
+        let some_str = String::from("/some/valid/utf8");
+        let some_dir = std::path::Path::new(&some_str);
+        assert_eq!(
+            some_dir.as_os_str().to_c_string().unwrap(),
+            CString::new("/some/valid/utf8").unwrap()
+        );
+    }
+
+    #[test]
+    fn test_os_str_to_c_string_unicode_fn() {
+        let some_str = std::ffi::OsString::from("foo");
+        assert_eq!(
+            os_str_to_c_string_unicode(&some_str).unwrap(),
+            CString::new("foo").unwrap()
+        );
+    }
+
+    #[test]
+    fn test_path_to_c_string_unicode_fn() {
+        let some_str = String::from("/some/path");
+        let some_path = std::path::Path::new(&some_str);
+        assert_eq!(
+            os_str_to_c_string_unicode(&some_path).unwrap(),
+            CString::new("/some/path").unwrap()
+        );
+    }
+
+    #[test]
+    fn test_os_str_to_c_string_unicode_fn_nul() {
+        let some_str = std::ffi::OsString::from("fooz\x00bar");
+        assert_eq!(
+            os_str_to_c_string_unicode(&some_str).err().unwrap(),
+            CStringError::InteriorNullByte
+        );
+    }
+
+    #[test]
+    fn test_as_path() {
+        let some_path = CString::new("/some/path").unwrap();
+        let ptr = some_path.as_ptr();
+        assert_eq!(as_path(ptr), std::ffi::OsString::from("/some/path"))
+    }
+
+    #[test]
+    fn test_as_path_unicode_fn() {
+        let some_path = CString::new("/some/path").unwrap();
+        let ptr = some_path.as_ptr();
+        assert_eq!(as_path_unicode(ptr), std::ffi::OsString::from("/some/path"));
+    }
+
+    #[test]
+    fn test_cstring_yolo() {
+        assert_eq!(CString::new("hello").unwrap(), CString::yolo("hello"));
+    }
+
+    #[test]
+    fn test_strdup_str() {
+        unsafe {
+            let s = "hello".strdup();
+            let cmp = strcmp(s, b"hello\x00" as *const u8 as *const libc::c_char);
+            free(s as *mut libc::c_void);
+            assert_eq!(cmp, 0);
+        }
+    }
+
+    #[test]
+    fn test_strdup_string() {
+        unsafe {
+            let s = String::from("hello").strdup();
+            let cmp = strcmp(s, b"hello\x00" as *const u8 as *const libc::c_char);
+            free(s as *mut libc::c_void);
+            assert_eq!(cmp, 0);
+        }
+    }
+}

examples/repl/cmdline.rs

@@ -19,12 +19,11 @@ use deltachat::peerstate::*;
 use deltachat::qr::*;
 use deltachat::sql;
 use deltachat::Event;
-use libc::free;
 
 /// Reset database tables. This function is called from Core cmdline.
 /// Argument is a bitmask, executing single or multiple actions in one call.
 /// e.g. bitmask 7 triggers actions definded with bits 1, 2 and 4.
-pub unsafe fn dc_reset_tables(context: &Context, bits: i32) -> i32 {
+pub fn dc_reset_tables(context: &Context, bits: i32) -> i32 {
     info!(context, "Resetting tables ({})...", bits);
     if 0 != bits & 1 {
         sql::execute(context, &context.sql, "DELETE FROM jobs;", params![]).unwrap();
@@ -107,18 +106,19 @@ fn dc_poke_eml_file(context: &Context, filename: impl AsRef<Path>) -> Result<(),
 /// @param context The context as created by dc_context_new().
 /// @param spec The file or directory to import. NULL for the last command.
 /// @return 1=success, 0=error.
-fn poke_spec(context: &Context, spec: *const libc::c_char) -> libc::c_int {
+fn poke_spec(context: &Context, spec: Option<&str>) -> libc::c_int {
     if !context.sql.is_open() {
         error!(context, "Import: Database not opened.");
         return 0;
     }
 
-    let real_spec: String;
     let mut read_cnt = 0;
 
+    let real_spec: String;
+
     /* if `spec` is given, remember it for later usage; if it is not given, try to use the last one */
-    if !spec.is_null() {
+    if let Some(spec) = spec {
-        real_spec = to_string_lossy(spec);
+        real_spec = spec.to_string();
         context
             .sql
             .set_raw_config(context, "import_spec", Some(&real_spec))
@@ -174,7 +174,7 @@ fn poke_spec(context: &Context, spec: *const libc::c_char) -> libc::c_int {
     1
 }
 
-unsafe fn log_msg(context: &Context, prefix: impl AsRef<str>, msg: &Message) {
+fn log_msg(context: &Context, prefix: impl AsRef<str>, msg: &Message) {
     let contact = Contact::get_by_id(context, msg.get_from_id()).expect("invalid contact");
     let contact_name = contact.get_name();
     let contact_id = contact.get_id();
@@ -219,7 +219,7 @@ unsafe fn log_msg(context: &Context, prefix: impl AsRef<str>, msg: &Message) {
     );
 }
 
-unsafe fn log_msglist(context: &Context, msglist: &Vec<MsgId>) -> Result<(), Error> {
+fn log_msglist(context: &Context, msglist: &Vec<MsgId>) -> Result<(), Error> {
     let mut lines_out = 0;
     for &msg_id in msglist {
         if msg_id.is_daymarker() {
@@ -250,7 +250,7 @@ unsafe fn log_msglist(context: &Context, msglist: &Vec<MsgId>) -> Result<(), Err
     Ok(())
 }
 
-unsafe fn log_contactlist(context: &Context, contacts: &Vec<u32>) {
+fn log_contactlist(context: &Context, contacts: &Vec<u32>) {
     let mut contacts = contacts.clone();
     if !contacts.contains(&1) {
         contacts.push(1);
@@ -302,7 +302,7 @@ fn chat_prefix(chat: &Chat) -> &'static str {
     chat.typ.into()
 }
 
-pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::Error> {
+pub fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::Error> {
     let chat_id = *context.cmdline_sel_chat_id.read().unwrap();
     let mut sel_chat = if chat_id > 0 {
         Chat::load_from_db(context, chat_id).ok()
@@ -313,11 +313,6 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
     let mut args = line.splitn(3, ' ');
     let arg0 = args.next().unwrap_or_default();
     let arg1 = args.next().unwrap_or_default();
-    let arg1_c = if arg1.is_empty() {
-        std::ptr::null()
-    } else {
-        arg1.strdup() as *const _
-    };
     let arg2 = args.next().unwrap_or_default();
 
     let blobdir = context.get_blobdir();
@@ -467,7 +462,7 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
             );
         }
         "poke" => {
-            ensure!(0 != poke_spec(context, arg1_c), "Poke failed");
+            ensure!(0 != poke_spec(context, Some(arg1)), "Poke failed");
         }
         "reset" => {
             ensure!(!arg1.is_empty(), "Argument <bits> missing: 1=jobs, 2=peerstates, 4=private keys, 8=rest but server config");
@@ -1008,7 +1003,5 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
         _ => bail!("Unknown command: \"{}\" type ? for help.", arg0),
     }
 
-    free(arg1_c as *mut _);
-
     Ok(())
 }

examples/repl/main.rs

@@ -403,7 +403,7 @@ fn main_0(args: Vec<String>) -> Result<(), failure::Error> {
                 // TODO: ignore "set mail_pw"
                 rl.add_history_entry(line.as_str());
                 let ctx = ctx.clone();
-                match unsafe { handle_cmd(line.trim(), ctx) } {
+                match handle_cmd(line.trim(), ctx) {
                     Ok(ExitResult::Continue) => {}
                     Ok(ExitResult::Exit) => break,
                     Err(err) => println!("Error: {}", err),
@@ -434,7 +434,7 @@ enum ExitResult {
     Exit,
 }
 
-unsafe fn handle_cmd(line: &str, ctx: Arc<RwLock<Context>>) -> Result<ExitResult, failure::Error> {
+fn handle_cmd(line: &str, ctx: Arc<RwLock<Context>>) -> Result<ExitResult, failure::Error> {
     let mut args = line.splitn(2, ' ');
     let arg0 = args.next().unwrap_or_default();
     let arg1 = args.next().unwrap_or_default();

examples/simple.rs

@@ -101,16 +101,6 @@ fn main() {
 
     thread::sleep(duration);
 
-    // let msglist = dc_get_chat_msgs(&ctx, chat_id, 0, 0);
-    // for i in 0..dc_array_get_cnt(msglist) {
-    //     let msg_id = dc_array_get_id(msglist, i);
-    //     let msg = dc_get_msg(context, msg_id);
-    //     let text = CStr::from_ptr(dc_msg_get_text(msg)).unwrap();
-    //     println!("Message {}: {}\n", i + 1, text.to_str().unwrap());
-    //     dc_msg_unref(msg);
-    // }
-    // dc_array_unref(msglist);
-
     println!("stopping threads");
 
     *running.write().unwrap() = false;

src/context.rs

@@ -86,7 +86,7 @@ pub fn get_info() -> HashMap<&'static str, String> {
     );
     res.insert(
         "arch",
-        (::std::mem::size_of::<*mut libc::c_void>())
+        (std::mem::size_of::<*mut libc::c_void>())
             .wrapping_mul(8)
             .to_string(),
     );

src/dc_receive_imf.rs

@@ -286,7 +286,7 @@ fn add_parts(
     sent_timestamp: &mut i64,
     from_id: &mut u32,
     from_id_blocked: i32,
-    hidden: &mut libc::c_int,
+    hidden: &mut i32,
     chat_id: &mut u32,
     to_id: &mut u32,
     flags: u32,
@@ -297,7 +297,7 @@ fn add_parts(
     create_event_to_send: &mut Option<CreateEvent>,
 ) -> Result<()> {
     let mut state: MessageState;
-    let mut msgrmsg: libc::c_int;
+    let mut msgrmsg: i32;
     let mut chat_id_blocked = Blocked::Not;
     let mut sort_timestamp = 0;
     let mut rcvd_timestamp = 0;
@@ -640,10 +640,10 @@ fn add_parts(
                 stmt.execute(params![
                     rfc724_mid,
                     server_folder.as_ref(),
-                    server_uid as libc::c_int,
+                    server_uid as i32,
-                    *chat_id as libc::c_int,
+                    *chat_id as i32,
-                    *from_id as libc::c_int,
+                    *from_id as i32,
-                    *to_id as libc::c_int,
+                    *to_id as i32,
                     sort_timestamp,
                     *sent_timestamp,
                     rcvd_timestamp,
@@ -754,7 +754,7 @@ fn calc_timestamps(
     chat_id: u32,
     from_id: u32,
     message_timestamp: i64,
-    is_fresh_msg: libc::c_int,
+    is_fresh_msg: i32,
     sort_timestamp: &mut i64,
     sent_timestamp: &mut i64,
     rcvd_timestamp: &mut i64,
@@ -799,7 +799,7 @@ fn calc_timestamps(
 fn create_or_lookup_group(
     context: &Context,
     mime_parser: &mut MimeParser,
-    allow_creation: libc::c_int,
+    allow_creation: i32,
     create_blocked: Blocked,
     from_id: u32,
     to_ids: &mut Vec<u32>,
@@ -889,7 +889,7 @@ fn create_or_lookup_group(
         X_MrRemoveFromGrp = Some(optional_field);
         mime_parser.is_system_message = SystemMessage::MemberRemovedFromGroup;
         let left_group = (Contact::lookup_id_by_addr(context, X_MrRemoveFromGrp.as_ref().unwrap())
-            == from_id as u32) as libc::c_int;
+            == from_id as u32) as i32;
         better_msg = context.stock_system_msg(
             if 0 != left_group {
                 StockMessage::MsgGroupLeft
@@ -1167,7 +1167,7 @@ fn create_or_lookup_group(
 fn create_or_lookup_adhoc_group(
     context: &Context,
     mime_parser: &MimeParser,
-    allow_creation: libc::c_int,
+    allow_creation: i32,
     create_blocked: Blocked,
     from_id: u32,
     to_ids: &mut Vec<u32>,
@@ -1252,7 +1252,7 @@ fn create_or_lookup_adhoc_group(
     let grpname = if let Some(subject) = mime_parser.subject.as_ref().filter(|s| !s.is_empty()) {
         subject.to_string()
     } else {
-        context.stock_string_repl_int(StockMessage::Member, member_ids.len() as libc::c_int)
+        context.stock_string_repl_int(StockMessage::Member, member_ids.len() as i32)
     };
 
     // create group record
@@ -1516,7 +1516,7 @@ fn set_better_msg(mime_parser: &mut MimeParser, better_msg: impl AsRef<str>) {
     }
 }
 
-fn dc_is_reply_to_known_message(context: &Context, mime_parser: &MimeParser) -> libc::c_int {
+fn dc_is_reply_to_known_message(context: &Context, mime_parser: &MimeParser) -> i32 {
     /* check if the message is a reply to a known message; the replies are identified by the Message-ID from
     `In-Reply-To`/`References:` (to support non-Delta-Clients) */
 
@@ -1566,7 +1566,7 @@ fn is_known_rfc724_mid(context: &Context, rfc724_mid: &mailparse::MailAddr) -> b
         .unwrap_or_default()
 }
 
-fn dc_is_reply_to_messenger_message(context: &Context, mime_parser: &MimeParser) -> libc::c_int {
+fn dc_is_reply_to_messenger_message(context: &Context, mime_parser: &MimeParser) -> i32 {
     /* function checks, if the message defined by mime_parser references a message send by us from Delta Chat.
     This is similar to is_reply_to_known_message() but
     - checks also if any of the referenced IDs are send by a messenger

src/dc_tools.rs

@@ -3,52 +3,22 @@
 
 use core::cmp::{max, min};
 use std::borrow::Cow;
-use std::ffi::{CStr, CString};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::time::SystemTime;
 use std::{fmt, fs};
 
 use chrono::{Local, TimeZone};
-use libc::{memcpy, strlen};
 use rand::{thread_rng, Rng};
 
 use crate::context::Context;
 use crate::error::Error;
 use crate::events::Event;
 
-pub(crate) fn dc_exactly_one_bit_set(v: libc::c_int) -> bool {
+pub(crate) fn dc_exactly_one_bit_set(v: i32) -> bool {
     0 != v && 0 == v & (v - 1)
 }
 
-/// Duplicates a string
-///
-/// returns an empty string if NULL is given, never returns NULL (exits on errors)
-///
-/// # Examples
-///
-/// ```
-/// use deltachat::dc_tools::{dc_strdup, to_string_lossy};
-/// unsafe {
-///     let str_a = b"foobar\x00" as *const u8 as *const libc::c_char;
-///     let str_a_copy = dc_strdup(str_a);
-///     assert_eq!(to_string_lossy(str_a_copy), "foobar");
-///     assert_ne!(str_a, str_a_copy);
-/// }
-/// ```
-pub unsafe fn dc_strdup(s: *const libc::c_char) -> *mut libc::c_char {
-    let ret: *mut libc::c_char;
-    if !s.is_null() {
-        ret = strdup(s);
-        assert!(!ret.is_null());
-    } else {
-        ret = libc::calloc(1, 1) as *mut libc::c_char;
-        assert!(!ret.is_null());
-    }
-
-    ret
-}
-
 /// Shortens a string to a specified length and adds "..." or "[...]" to the end of
 /// the shortened string.
 pub(crate) fn dc_truncate(buf: &str, approx_chars: usize, do_unwrap: bool) -> Cow<str> {
@@ -532,212 +502,6 @@ pub(crate) fn dc_get_next_backup_path(
     bail!("could not create backup file, disk full?");
 }
 
-/// Error type for the [OsStrExt] trait
-#[derive(Debug, Fail, PartialEq)]
-pub enum CStringError {
-    /// The string contains an interior null byte
-    #[fail(display = "String contains an interior null byte")]
-    InteriorNullByte,
-    /// The string is not valid Unicode
-    #[fail(display = "String is not valid unicode")]
-    NotUnicode,
-}
-
-/// Extra convenience methods on [std::ffi::OsStr] to work with `*libc::c_char`.
-///
-/// The primary function of this trait is to more easily convert
-/// [OsStr], [OsString] or [Path] into pointers to C strings.  This always
-/// allocates a new string since it is very common for the source
-/// string not to have the required terminal null byte.
-///
-/// It is implemented for `AsRef<std::ffi::OsStr>>` trait, which
-/// allows any type which implements this trait to transparently use
-/// this.  This is how the conversion for [Path] works.
-///
-/// [OsStr]: std::ffi::OsStr
-/// [OsString]: std::ffi::OsString
-/// [Path]: std::path::Path
-///
-/// # Example
-///
-/// ```
-/// use deltachat::dc_tools::{dc_strdup, OsStrExt};
-/// let path = std::path::Path::new("/some/path");
-/// let path_c = path.to_c_string().unwrap();
-/// unsafe {
-///     let mut c_ptr: *mut libc::c_char = dc_strdup(path_c.as_ptr());
-/// }
-/// ```
-pub trait OsStrExt {
-    /// Convert a  [std::ffi::OsStr] to an [std::ffi::CString]
-    ///
-    /// This is useful to convert e.g. a [std::path::Path] to
-    /// [*libc::c_char] by using
-    /// [Path::as_os_str()](std::path::Path::as_os_str) and
-    /// [CStr::as_ptr()](std::ffi::CStr::as_ptr).
-    ///
-    /// This returns [CString] and not [&CStr] because not all [OsStr]
-    /// slices end with a null byte, particularly those coming from
-    /// [Path] do not have a null byte and having to handle this as
-    /// the caller would defeat the point of this function.
-    ///
-    /// On Windows this requires that the [OsStr] contains valid
-    /// unicode, which should normally be the case for a [Path].
-    ///
-    /// [CString]: std::ffi::CString
-    /// [CStr]: std::ffi::CStr
-    /// [OsStr]: std::ffi::OsStr
-    /// [Path]: std::path::Path
-    ///
-    /// # Errors
-    ///
-    /// Since a C `*char` is terminated by a NULL byte this conversion
-    /// will fail, when the [OsStr] has an interior null byte.  The
-    /// function will return
-    /// `[Err]([CStringError::InteriorNullByte])`.  When converting
-    /// from a [Path] it should be safe to
-    /// [`.unwrap()`](std::result::Result::unwrap) this anyway since a
-    /// [Path] should not contain interior null bytes.
-    ///
-    /// On windows when the string contains invalid Unicode
-    /// `[Err]([CStringError::NotUnicode])` is returned.
-    fn to_c_string(&self) -> Result<CString, CStringError>;
-}
-
-impl<T: AsRef<std::ffi::OsStr>> OsStrExt for T {
-    #[cfg(not(target_os = "windows"))]
-    fn to_c_string(&self) -> Result<CString, CStringError> {
-        use std::os::unix::ffi::OsStrExt;
-        CString::new(self.as_ref().as_bytes()).map_err(|err| match err {
-            std::ffi::NulError { .. } => CStringError::InteriorNullByte,
-        })
-    }
-
-    #[cfg(target_os = "windows")]
-    fn to_c_string(&self) -> Result<CString, CStringError> {
-        os_str_to_c_string_unicode(&self)
-    }
-}
-
-// Implementation for os_str_to_c_string on windows.
-#[allow(dead_code)]
-fn os_str_to_c_string_unicode(
-    os_str: &dyn AsRef<std::ffi::OsStr>,
-) -> Result<CString, CStringError> {
-    match os_str.as_ref().to_str() {
-        Some(val) => CString::new(val.as_bytes()).map_err(|err| match err {
-            std::ffi::NulError { .. } => CStringError::InteriorNullByte,
-        }),
-        None => Err(CStringError::NotUnicode),
-    }
-}
-
-/// Convenience methods/associated functions for working with [CString]
-///
-/// This is helps transitioning from unsafe code.
-pub trait CStringExt {
-    /// Create a new [CString], yolo style
-    ///
-    /// This unwrap the result, panicking when there are embedded NULL
-    /// bytes.
-    fn yolo<T: Into<Vec<u8>>>(t: T) -> CString {
-        CString::new(t).expect("String contains null byte, can not be CString")
-    }
-}
-
-impl CStringExt for CString {}
-
-/// Convenience methods to make transitioning from raw C strings easier.
-///
-/// To interact with (legacy) C APIs we often need to convert from
-/// Rust strings to raw C strings.  This can be clumsy to do correctly
-/// and the compiler sometimes allows it in an unsafe way.  These
-/// methods make it more succinct and help you get it right.
-pub trait StrExt {
-    /// Allocate a new raw C `*char` version of this string.
-    ///
-    /// This allocates a new raw C string which must be freed using
-    /// `free`.  It takes care of some common pitfalls with using
-    /// [CString.as_ptr].
-    ///
-    /// [CString.as_ptr]: std::ffi::CString.as_ptr
-    ///
-    /// # Panics
-    ///
-    /// This function will panic when the original string contains an
-    /// interior null byte as this can not be represented in raw C
-    /// strings.
-    unsafe fn strdup(&self) -> *mut libc::c_char;
-}
-
-impl<T: AsRef<str>> StrExt for T {
-    unsafe fn strdup(&self) -> *mut libc::c_char {
-        let tmp = CString::yolo(self.as_ref());
-        dc_strdup(tmp.as_ptr())
-    }
-}
-
-pub fn to_string_lossy(s: *const libc::c_char) -> String {
-    if s.is_null() {
-        return "".into();
-    }
-
-    let cstr = unsafe { CStr::from_ptr(s) };
-
-    cstr.to_string_lossy().to_string()
-}
-
-pub fn to_opt_string_lossy(s: *const libc::c_char) -> Option<String> {
-    if s.is_null() {
-        return None;
-    }
-
-    Some(to_string_lossy(s))
-}
-
-/// Convert a C `*char` pointer to a [std::path::Path] slice.
-///
-/// This converts a `*libc::c_char` pointer to a [Path] slice.  This
-/// essentially has to convert the pointer to [std::ffi::OsStr] to do
-/// so and thus is the inverse of [OsStrExt::to_c_string].  Just like
-/// [OsStrExt::to_c_string] requires valid Unicode on Windows, this
-/// requires that the pointer contains valid UTF-8 on Windows.
-///
-/// Because this returns a reference the [Path] silce can not outlive
-/// the original pointer.
-///
-/// [Path]: std::path::Path
-#[cfg(not(target_os = "windows"))]
-pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path {
-    assert!(!s.is_null(), "cannot be used on null pointers");
-    use std::os::unix::ffi::OsStrExt;
-    unsafe {
-        let c_str = std::ffi::CStr::from_ptr(s).to_bytes();
-        let os_str = std::ffi::OsStr::from_bytes(c_str);
-        std::path::Path::new(os_str)
-    }
-}
-
-// as_path() implementation for windows, documented above.
-#[cfg(target_os = "windows")]
-pub fn as_path<'a>(s: *const libc::c_char) -> &'a std::path::Path {
-    as_path_unicode(s)
-}
-
-// Implementation for as_path() on Windows.
-//
-// Having this as a separate function means it can be tested on unix
-// too.
-#[allow(dead_code)]
-fn as_path_unicode<'a>(s: *const libc::c_char) -> &'a std::path::Path {
-    assert!(!s.is_null(), "cannot be used on null pointers");
-
-    let cstr = unsafe { CStr::from_ptr(s) };
-    let str = cstr.to_str().unwrap_or_else(|err| panic!("{}", err));
-
-    std::path::Path::new(str)
-}
-
 pub(crate) fn time() -> i64 {
     SystemTime::now()
         .duration_since(SystemTime::UNIX_EPOCH)
@@ -815,57 +579,14 @@ pub(crate) fn listflags_has(listflags: u32, bitindex: usize) -> bool {
     (listflags & bitindex) == bitindex
 }
 
-pub(crate) unsafe fn strdup(s: *const libc::c_char) -> *mut libc::c_char {
-    if s.is_null() {
-        return std::ptr::null_mut();
-    }
-
-    let slen = strlen(s);
-    let result = libc::malloc(slen + 1);
-    if result.is_null() {
-        return std::ptr::null_mut();
-    }
-
-    memcpy(result, s as *const _, slen + 1);
-    result as *mut _
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
-
-    use libc::{free, strcmp};
     use std::convert::TryInto;
-    use std::ffi::CStr;
 
     use crate::constants::*;
     use crate::test_utils::*;
 
-    #[test]
-    fn test_dc_strdup() {
-        unsafe {
-            let str_a = b"foobar\x00" as *const u8 as *const libc::c_char;
-            let str_a_copy = dc_strdup(str_a);
-
-            // Value of str_a_copy should equal foobar
-            assert_eq!(
-                CStr::from_ptr(str_a_copy),
-                CString::new("foobar").unwrap().as_c_str()
-            );
-            // Address of str_a should be different from str_a_copy
-            assert_ne!(str_a, str_a_copy);
-
-            let str_a = std::ptr::null() as *const libc::c_char;
-            let str_a_copy = dc_strdup(str_a);
-            // Value of str_a_copy should equal ""
-            assert_eq!(
-                CStr::from_ptr(str_a_copy),
-                CString::new("").unwrap().as_c_str()
-            );
-            assert_ne!(str_a, str_a_copy);
-        }
-    }
-
     #[test]
     fn test_rust_ftoa() {
         assert_eq!("1.22", format!("{}", 1.22));
@@ -953,113 +674,6 @@ mod tests {
     }
 
     #[test]
-    fn test_os_str_to_c_string_cwd() {
-        let some_dir = std::env::current_dir().unwrap();
-        some_dir.as_os_str().to_c_string().unwrap();
-    }
-
-    #[test]
-    fn test_os_str_to_c_string_unicode() {
-        let some_str = String::from("/some/valid/utf8");
-        let some_dir = std::path::Path::new(&some_str);
-        assert_eq!(
-            some_dir.as_os_str().to_c_string().unwrap(),
-            CString::new("/some/valid/utf8").unwrap()
-        );
-    }
-
-    #[test]
-    fn test_os_str_to_c_string_nul() {
-        let some_str = std::ffi::OsString::from("foo\x00bar");
-        assert_eq!(
-            some_str.to_c_string().err().unwrap(),
-            CStringError::InteriorNullByte
-        )
-    }
-
-    #[test]
-    fn test_path_to_c_string_cwd() {
-        let some_dir = std::env::current_dir().unwrap();
-        some_dir.to_c_string().unwrap();
-    }
-
-    #[test]
-    fn test_path_to_c_string_unicode() {
-        let some_str = String::from("/some/valid/utf8");
-        let some_dir = std::path::Path::new(&some_str);
-        assert_eq!(
-            some_dir.as_os_str().to_c_string().unwrap(),
-            CString::new("/some/valid/utf8").unwrap()
-        );
-    }
-
-    #[test]
-    fn test_os_str_to_c_string_unicode_fn() {
-        let some_str = std::ffi::OsString::from("foo");
-        assert_eq!(
-            os_str_to_c_string_unicode(&some_str).unwrap(),
-            CString::new("foo").unwrap()
-        );
-    }
-
-    #[test]
-    fn test_path_to_c_string_unicode_fn() {
-        let some_str = String::from("/some/path");
-        let some_path = std::path::Path::new(&some_str);
-        assert_eq!(
-            os_str_to_c_string_unicode(&some_path).unwrap(),
-            CString::new("/some/path").unwrap()
-        );
-    }
-
-    #[test]
-    fn test_os_str_to_c_string_unicode_fn_nul() {
-        let some_str = std::ffi::OsString::from("fooz\x00bar");
-        assert_eq!(
-            os_str_to_c_string_unicode(&some_str).err().unwrap(),
-            CStringError::InteriorNullByte
-        );
-    }
-
-    #[test]
-    fn test_as_path() {
-        let some_path = CString::new("/some/path").unwrap();
-        let ptr = some_path.as_ptr();
-        assert_eq!(as_path(ptr), std::ffi::OsString::from("/some/path"))
-    }
-
-    #[test]
-    fn test_as_path_unicode_fn() {
-        let some_path = CString::new("/some/path").unwrap();
-        let ptr = some_path.as_ptr();
-        assert_eq!(as_path_unicode(ptr), std::ffi::OsString::from("/some/path"));
-    }
-
-    #[test]
-    fn test_cstring_yolo() {
-        assert_eq!(CString::new("hello").unwrap(), CString::yolo("hello"));
-    }
-
-    #[test]
-    fn test_strdup_str() {
-        unsafe {
-            let s = "hello".strdup();
-            let cmp = strcmp(s, b"hello\x00" as *const u8 as *const libc::c_char);
-            free(s as *mut libc::c_void);
-            assert_eq!(cmp, 0);
-        }
-    }
-
-    #[test]
-    fn test_strdup_string() {
-        unsafe {
-            let s = String::from("hello").strdup();
-            let cmp = strcmp(s, b"hello\x00" as *const u8 as *const libc::c_char);
-            free(s as *mut libc::c_void);
-            assert_eq!(cmp, 0);
-        }
-    }
-
     #[test]
     fn test_dc_extract_grpid_from_rfc724_mid() {
         // Should return None if we pass invalid mid

src/error.rs

@@ -15,8 +15,6 @@ pub enum Error {
     Image(image_meta::ImageError),
     #[fail(display = "{:?}", _0)]
     Utf8(std::str::Utf8Error),
-    #[fail(display = "{:?}", _0)]
-    CStringError(crate::dc_tools::CStringError),
     #[fail(display = "PGP: {:?}", _0)]
     Pgp(pgp::errors::Error),
     #[fail(display = "Base64Decode: {:?}", _0)]
@@ -75,12 +73,6 @@ impl From<image_meta::ImageError> for Error {
     }
 }
 
-impl From<crate::dc_tools::CStringError> for Error {
-    fn from(err: crate::dc_tools::CStringError) -> Error {
-        Error::CStringError(err)
-    }
-}
-
 impl From<pgp::errors::Error> for Error {
     fn from(err: pgp::errors::Error) -> Error {
         Error::Pgp(err)

src/job.rs

@@ -898,7 +898,7 @@ fn add_smtp_job(
 pub fn job_add(
     context: &Context,
     action: Action,
-    foreign_id: libc::c_int,
+    foreign_id: i32,
     param: Params,
     delay_seconds: i64,
 ) {

src/location.rs

@@ -229,7 +229,7 @@ pub fn send_locations_to_chat(context: &Context, chat_id: u32, seconds: i64) {
                 job_add(
                     context,
                     Action::MaybeSendLocationsEnded,
-                    chat_id as libc::c_int,
+                    chat_id as i32,
                     Params::new(),
                     seconds + 1,
                 );

src/message.rs

@@ -457,7 +457,7 @@ impl Message {
             return ret;
         };
 
-        let contact = if self.from_id != DC_CONTACT_ID_SELF as libc::c_uint
+        let contact = if self.from_id != DC_CONTACT_ID_SELF as u32
             && (chat.typ == Chattype::Group || chat.typ == Chattype::VerifiedGroup)
         {
             Contact::get_by_id(context, self.from_id).ok()
@@ -502,8 +502,8 @@ impl Message {
 
     pub fn is_info(&self) -> bool {
         let cmd = self.param.get_cmd();
-        self.from_id == DC_CONTACT_ID_INFO as libc::c_uint
+        self.from_id == DC_CONTACT_ID_INFO as u32
-            || self.to_id == DC_CONTACT_ID_INFO as libc::c_uint
+            || self.to_id == DC_CONTACT_ID_INFO as u32
             || cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage
     }
 
@@ -730,7 +730,7 @@ pub fn get_msg_info(context: &Context, msg_id: MsgId) -> String {
     ret += &format!(" by {}", name);
     ret += "\n";
 
-    if msg.from_id != DC_CONTACT_ID_SELF as libc::c_uint {
+    if msg.from_id != DC_CONTACT_ID_SELF as u32 {
         let s = dc_timestamp_to_str(if 0 != msg.timestamp_rcvd {
             msg.timestamp_rcvd
         } else {
@@ -1227,7 +1227,7 @@ pub fn mdn_from_ext(
 }
 
 /// The number of messages assigned to real chat (!=deaddrop, !=trash)
-pub fn get_real_msg_cnt(context: &Context) -> libc::c_int {
+pub fn get_real_msg_cnt(context: &Context) -> i32 {
     match context.sql.query_row(
         "SELECT COUNT(*) \
          FROM msgs m  LEFT JOIN chats c ON c.id=m.chat_id \
@@ -1243,7 +1243,7 @@ pub fn get_real_msg_cnt(context: &Context) -> libc::c_int {
     }
 }
 
-pub fn get_deaddrop_msg_cnt(context: &Context) -> libc::size_t {
+pub fn get_deaddrop_msg_cnt(context: &Context) -> usize {
     match context.sql.query_row(
         "SELECT COUNT(*) \
          FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id \
@@ -1251,7 +1251,7 @@ pub fn get_deaddrop_msg_cnt(context: &Context) -> libc::size_t {
         rusqlite::NO_PARAMS,
         |row| row.get::<_, isize>(0),
     ) {
-        Ok(res) => res as libc::size_t,
+        Ok(res) => res as usize,
         Err(err) => {
             error!(context, "dc_get_deaddrop_msg_cnt() failed. {}", err);
             0
@@ -1259,7 +1259,7 @@ pub fn get_deaddrop_msg_cnt(context: &Context) -> libc::size_t {
     }
 }
 
-pub fn rfc724_mid_cnt(context: &Context, rfc724_mid: &str) -> libc::c_int {
+pub fn rfc724_mid_cnt(context: &Context, rfc724_mid: &str) -> i32 {
     // check the number of messages with the same rfc724_mid
     match context.sql.query_row(
         "SELECT COUNT(*) FROM msgs WHERE rfc724_mid=?;",

src/mimeparser.rs

@@ -232,7 +232,7 @@ impl<'a> MimeParser<'a> {
             }
         }
         if let Some(ref subject) = self.subject {
-            let mut prepend_subject: libc::c_int = 1i32;
+            let mut prepend_subject = 1i32;
             if !self.decrypting_failed {
                 let colon = subject.find(':');
                 if colon == Some(2)

tests/stress.rs

@@ -14,7 +14,7 @@ use tempfile::{tempdir, TempDir};
 /* some data used for testing
  ******************************************************************************/
 
-unsafe fn stress_functions(context: &Context) {
+fn stress_functions(context: &Context) {
     let res = context.get_config(config::Config::SysConfigKeys).unwrap();
 
     assert!(!res.contains(" probably_never_a_key "));
@@ -224,10 +224,8 @@ fn create_test_context() -> TestContext {
 
 #[test]
 fn test_stress_tests() {
-    unsafe {
+    let context = create_test_context();
-        let context = create_test_context();
+    stress_functions(&context.ctx);
-        stress_functions(&context.ctx);
-    }
 }
 
 #[test]