diff --git a/Cargo.lock b/Cargo.lock index 12ae271f5..152e9dcb1 100644 --- a/Cargo.lock +++ b/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)", diff --git a/deltachat-ffi/Cargo.toml b/deltachat-ffi/Cargo.toml index 39f293366..c154d924a 100644 --- a/deltachat-ffi/Cargo.toml +++ b/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"] diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index f1f8697b8..3da0e0e3c 100644 --- a/deltachat-ffi/src/lib.rs +++ b/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 diff --git a/deltachat-ffi/src/providers.rs b/deltachat-ffi/src/providers.rs index ceaa0d5ae..42c169c77 100644 --- a/deltachat-ffi/src/providers.rs +++ b/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] diff --git a/deltachat-ffi/src/string.rs b/deltachat-ffi/src/string.rs new file mode 100644 index 000000000..8baf4d56a --- /dev/null +++ b/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>` 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; +} + +impl> OsStrExt for T { + #[cfg(not(target_os = "windows"))] + fn to_c_string(&self) -> Result { + 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 { + 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, +) -> Result { + 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: 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> 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 { + 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); + } + } +} diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index f755fde82..916e3decf 100644 --- a/examples/repl/cmdline.rs +++ b/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) -> 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() { - real_spec = to_string_lossy(spec); + if let Some(spec) = 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, msg: &Message) { +fn log_msg(context: &Context, prefix: impl AsRef, 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, msg: &Message) { ); } -unsafe fn log_msglist(context: &Context, msglist: &Vec) -> Result<(), Error> { +fn log_msglist(context: &Context, msglist: &Vec) -> 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) -> Result<(), Err Ok(()) } -unsafe fn log_contactlist(context: &Context, contacts: &Vec) { +fn log_contactlist(context: &Context, contacts: &Vec) { 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 missing: 1=jobs, 2=peerstates, 4=private keys, 8=rest but server config"); @@ -1005,7 +1000,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(()) } diff --git a/examples/repl/main.rs b/examples/repl/main.rs index bc7528c5d..178f4fb0d 100644 --- a/examples/repl/main.rs +++ b/examples/repl/main.rs @@ -403,7 +403,7 @@ fn main_0(args: Vec) -> 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>) -> Result { +fn handle_cmd(line: &str, ctx: Arc>) -> Result { let mut args = line.splitn(2, ' '); let arg0 = args.next().unwrap_or_default(); let arg1 = args.next().unwrap_or_default(); diff --git a/examples/simple.rs b/examples/simple.rs index 67c8fbbc7..71917e7c1 100644 --- a/examples/simple.rs +++ b/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; diff --git a/src/context.rs b/src/context.rs index 6515b1842..711e34be1 100644 --- a/src/context.rs +++ b/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(), ); diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index b3d6fa2a6..ad559bb57 100644 --- a/src/dc_receive_imf.rs +++ b/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, ) -> 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, - *chat_id as libc::c_int, - *from_id as libc::c_int, - *to_id as libc::c_int, + server_uid as i32, + *chat_id as i32, + *from_id as i32, + *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, @@ -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, @@ -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) { } } -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 diff --git a/src/dc_tools.rs b/src/dc_tools.rs index 0e58b2a88..08f58296c 100644 --- a/src/dc_tools.rs +++ b/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 { @@ -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>` 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; -} - -impl> OsStrExt for T { - #[cfg(not(target_os = "windows"))] - fn to_c_string(&self) -> Result { - 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 { - 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, -) -> Result { - 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: 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> 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 { - 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 diff --git a/src/error.rs b/src/error.rs index 25351bf71..588117f45 100644 --- a/src/error.rs +++ b/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 for Error { } } -impl From for Error { - fn from(err: crate::dc_tools::CStringError) -> Error { - Error::CStringError(err) - } -} - impl From for Error { fn from(err: pgp::errors::Error) -> Error { Error::Pgp(err) diff --git a/src/job.rs b/src/job.rs index 7634e3a8b..ae545f620 100644 --- a/src/job.rs +++ b/src/job.rs @@ -915,7 +915,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, ) { diff --git a/src/location.rs b/src/location.rs index 357349d89..35bee16da 100644 --- a/src/location.rs +++ b/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, ); diff --git a/src/message.rs b/src/message.rs index 001a456b3..b12dd251f 100644 --- a/src/message.rs +++ b/src/message.rs @@ -431,7 +431,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() @@ -476,8 +476,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.to_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 u32 || cmd != SystemMessage::Unknown && cmd != SystemMessage::AutocryptSetupMessage } @@ -704,7 +704,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 { @@ -1201,7 +1201,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 \ @@ -1217,7 +1217,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 \ @@ -1225,7 +1225,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 @@ -1233,7 +1233,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=?;", diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 8c05cb4a5..8fe8deadd 100644 --- a/src/mimeparser.rs +++ b/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) diff --git a/tests/stress.rs b/tests/stress.rs index f03c9e2c8..9939766e1 100644 --- a/tests/stress.rs +++ b/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(); - stress_functions(&context.ctx); - } + let context = create_test_context(); + stress_functions(&context.ctx); } #[test]