diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 559917792..dfd43ec02 100644 --- a/deltachat-ffi/src/lib.rs +++ b/deltachat-ffi/src/lib.rs @@ -52,6 +52,9 @@ pub struct ContextWrapper { inner: RwLock>, } +unsafe impl Send for ContextWrapper {} +unsafe impl Sync for ContextWrapper {} + /// Callback function that should be given to [dc_context_new]. /// /// @memberof [dc_context_t] @@ -169,13 +172,13 @@ pub unsafe extern "C" fn dc_open( eprintln!("ignoring careless call to dc_open()"); return 0; } - let rust_cb = move |_ctx: &Context, evt: Event, d0: uintptr_t, d1: uintptr_t| { - let ffi_context = &*context; - match ffi_context.cb { + let ffi_context = &*context; + + let rust_cb = + move |_ctx: &Context, evt: Event, d0: uintptr_t, d1: uintptr_t| match ffi_context.cb { Some(ffi_cb) => ffi_cb(ffi_context, evt, d0, d1), None => 0, - } - }; + }; let ffi_context = &mut *context; let ctx = if blobdir.is_null() { Context::new( @@ -233,7 +236,7 @@ pub unsafe extern "C" fn dc_get_blobdir(context: *mut dc_context_t) -> *mut libc } let ffi_context = &*context; ffi_context - .with_inner(|ctx| context::dc_get_blobdir(ctx)) + .with_inner(|ctx| ctx.get_blobdir().to_string_lossy().strdup()) .unwrap_or_else(|_| "".strdup()) } @@ -1413,7 +1416,7 @@ pub unsafe extern "C" fn dc_imex( } let ffi_context = &*context; ffi_context - .with_inner(|ctx| dc_imex::dc_imex(ctx, what, param1, param2)) + .with_inner(|ctx| dc_imex::dc_imex(ctx, what, as_opt_str(param1), param2)) .ok(); } @@ -1428,7 +1431,7 @@ pub unsafe extern "C" fn dc_imex_has_backup( } let ffi_context = &*context; ffi_context - .with_inner(|ctx| dc_imex::dc_imex_has_backup(ctx, dir)) + .with_inner(|ctx| dc_imex::dc_imex_has_backup(ctx, as_str(dir))) .unwrap_or_else(|_| ptr::null_mut()) } diff --git a/examples/repl/cmdline.rs b/examples/repl/cmdline.rs index 30afcd589..f2c889d7d 100644 --- a/examples/repl/cmdline.rs +++ b/examples/repl/cmdline.rs @@ -491,17 +491,17 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E } } "export-backup" => { - dc_imex(context, 11, context.get_blobdir(), ptr::null()); + dc_imex(context, 11, Some(context.get_blobdir()), ptr::null()); } "import-backup" => { ensure!(!arg1.is_empty(), "Argument missing."); - dc_imex(context, 12, arg1_c, ptr::null()); + dc_imex(context, 12, Some(arg1), ptr::null()); } "export-keys" => { - dc_imex(context, 1, context.get_blobdir(), ptr::null()); + dc_imex(context, 1, Some(context.get_blobdir()), ptr::null()); } "import-keys" => { - dc_imex(context, 2, context.get_blobdir(), ptr::null()); + dc_imex(context, 2, Some(context.get_blobdir()), ptr::null()); } "export-setup" => { let setup_code = dc_create_setup_code(context); diff --git a/src/context.rs b/src/context.rs index 613a669c4..04133679b 100644 --- a/src/context.rs +++ b/src/context.rs @@ -35,12 +35,11 @@ use crate::x::*; /// /// This callback must return 0 unless stated otherwise in the event /// description at [Event]. -pub type ContextCallback = dyn Fn(&Context, Event, uintptr_t, uintptr_t) -> uintptr_t; +pub type ContextCallback = dyn Fn(&Context, Event, uintptr_t, uintptr_t) -> uintptr_t + Send + Sync; pub struct Context { - pub userdata: *mut libc::c_void, - pub dbfile: Arc>>, - pub blobdir: Arc>, + pub dbfile: Arc>, + pub blobdir: Arc>, pub sql: Sql, pub inbox: Arc>, pub perform_inbox_jobs_needed: Arc>, @@ -60,9 +59,6 @@ pub struct Context { pub generating_key_mutex: Mutex<()>, } -unsafe impl std::marker::Send for Context {} -unsafe impl std::marker::Sync for Context {} - #[derive(Debug, PartialEq, Eq)] pub struct RunningState { pub ongoing_running: bool, @@ -92,10 +88,9 @@ impl Context { "Blobdir does not exist: {}", blobdir.display() ); - let blobdir_c = blobdir.to_c_string()?; let ctx = Context { - blobdir: Arc::new(RwLock::new(unsafe { dc_strdup(blobdir_c.as_ptr()) })), - dbfile: Arc::new(RwLock::new(Some(dbfile))), + blobdir: Arc::new(RwLock::new(blobdir)), + dbfile: Arc::new(RwLock::new(dbfile)), inbox: Arc::new(RwLock::new({ Imap::new( cb_get_config, @@ -104,7 +99,6 @@ impl Context { cb_receive_imf, ) })), - userdata: std::ptr::null_mut(), cb, os_name: Some(os_name), running_state: Arc::new(RwLock::new(Default::default())), @@ -139,31 +133,18 @@ impl Context { perform_inbox_jobs_needed: Arc::new(RwLock::new(false)), generating_key_mutex: Mutex::new(()), }; - if !ctx - .sql - .open(&ctx, &ctx.dbfile.read().unwrap().as_ref().unwrap(), 0) - { + if !ctx.sql.open(&ctx, &ctx.dbfile.read().unwrap(), 0) { return Err(format_err!("Failed opening sqlite database")); } Ok(ctx) } - pub fn has_dbfile(&self) -> bool { - self.get_dbfile().is_some() + pub fn get_dbfile(&self) -> PathBuf { + self.dbfile.clone().read().unwrap().clone() } - pub fn has_blobdir(&self) -> bool { - !self.get_blobdir().is_null() - } - - pub fn get_dbfile(&self) -> Option { - (*self.dbfile.clone().read().unwrap()) - .as_ref() - .map(|x| x.clone()) - } - - pub fn get_blobdir(&self) -> *const libc::c_char { - *self.blobdir.clone().read().unwrap() + pub fn get_blobdir(&self) -> PathBuf { + self.blobdir.clone().read().unwrap().clone() } pub fn call_cb(&self, event: Event, data1: uintptr_t, data2: uintptr_t) -> uintptr_t { @@ -173,19 +154,15 @@ impl Context { impl Drop for Context { fn drop(&mut self) { - unsafe { - info!(self, "disconnecting INBOX-watch",); - self.inbox.read().unwrap().disconnect(self); - info!(self, "disconnecting sentbox-thread",); - self.sentbox_thread.read().unwrap().imap.disconnect(self); - info!(self, "disconnecting mvbox-thread",); - self.mvbox_thread.read().unwrap().imap.disconnect(self); - info!(self, "disconnecting SMTP"); - self.smtp.clone().lock().unwrap().disconnect(); - self.sql.close(self); - let blobdir = self.blobdir.write().unwrap(); - free(*blobdir as *mut libc::c_void); - } + info!(self, "disconnecting INBOX-watch",); + self.inbox.read().unwrap().disconnect(self); + info!(self, "disconnecting sentbox-thread",); + self.sentbox_thread.read().unwrap().imap.disconnect(self); + info!(self, "disconnecting mvbox-thread",); + self.mvbox_thread.read().unwrap().imap.disconnect(self); + info!(self, "disconnecting SMTP"); + self.smtp.clone().lock().unwrap().disconnect(); + self.sql.close(self); } } @@ -297,14 +274,6 @@ fn cb_get_config(context: &Context, key: &str) -> Option { context.sql.get_config(context, key) } -pub unsafe fn dc_get_userdata(context: &mut Context) -> *mut libc::c_void { - context.userdata as *mut _ -} - -pub unsafe fn dc_get_blobdir(context: &Context) -> *mut libc::c_char { - dc_strdup(*context.blobdir.clone().read().unwrap()) -} - /* ****************************************************************************** * INI-handling, Information ******************************************************************************/ @@ -421,16 +390,9 @@ pub unsafe fn dc_get_info(context: &Context) -> *mut libc::c_char { real_msgs, deaddrop_msgs, contacts, - context - .get_dbfile() - .as_ref() - .map_or(unset, |p| p.to_str().unwrap()), + context.get_dbfile().to_str().unwrap(), dbversion, - if context.has_blobdir() { - as_str(context.get_blobdir()) - } else { - unset - }, + context.get_blobdir().to_str().unwrap(), displayname.unwrap_or_else(|| unset.into()), is_configured, l, diff --git a/src/dc_imex.rs b/src/dc_imex.rs index 9ee239218..e40d9af27 100644 --- a/src/dc_imex.rs +++ b/src/dc_imex.rs @@ -1,4 +1,5 @@ use std::ffi::CString; +use std::path::Path; use std::ptr; use mmime::mailmime_content::*; @@ -32,13 +33,13 @@ use crate::x::*; pub fn dc_imex( context: &Context, what: libc::c_int, - param1: *const libc::c_char, + param1: Option>, param2: *const libc::c_char, ) { let mut param = Params::new(); param.set_int(Param::Cmd, what as i32); - if !param1.is_null() { - param.set(Param::Arg, as_str(param1)); + if let Some(param1) = param1 { + param.set(Param::Arg, param1.as_ref().to_string_lossy()); } if !param2.is_null() { param.set(Param::Arg2, as_str(param2)); @@ -51,9 +52,9 @@ pub fn dc_imex( /// Returns the filename of the backup if found, nullptr otherwise. pub unsafe fn dc_imex_has_backup( context: &Context, - dir_name: *const libc::c_char, + dir_name: impl AsRef, ) -> *mut libc::c_char { - let dir_name = as_path(dir_name); + let dir_name = dir_name.as_ref(); let dir_iter = std::fs::read_dir(dir_name); if dir_iter.is_err() { info!( @@ -567,10 +568,7 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char context, "Import \"{}\" to \"{}\".", as_str(backup_to_import), - context - .get_dbfile() - .as_ref() - .map_or("<>", |p| p.to_str().unwrap()) + context.get_dbfile().display() ); if dc_is_configured(context) { @@ -578,8 +576,8 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char return false; } &context.sql.close(&context); - dc_delete_file(context, context.get_dbfile().unwrap()); - if dc_file_exist(context, context.get_dbfile().unwrap()) { + dc_delete_file(context, context.get_dbfile()); + if dc_file_exist(context, context.get_dbfile()) { error!( context, "Cannot import backups: Cannot delete the old file.", @@ -587,19 +585,12 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char return false; } - if !dc_copy_file( - context, - as_path(backup_to_import), - context.get_dbfile().unwrap(), - ) { + if !dc_copy_file(context, as_path(backup_to_import), context.get_dbfile()) { return false; } /* error already logged */ /* re-open copied database file */ - if !context - .sql - .open(&context, &context.get_dbfile().unwrap(), 0) - { + if !context.sql.open(&context, &context.get_dbfile(), 0) { return false; } @@ -651,14 +642,14 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char continue; } - let pathNfilename = format!("{}/{}", as_str(context.get_blobdir()), file_name); + let pathNfilename = context.get_blobdir().join(file_name); if dc_write_file_safe(context, &pathNfilename, &file_blob) { continue; } error!( context, "Storage full? Cannot write file {} with {} bytes.", - &pathNfilename, + pathNfilename.display(), file_blob.len(), ); // otherwise the user may believe the stuff is imported correctly, but there are files missing ... @@ -715,20 +706,11 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { info!( context, "Backup \"{}\" to \"{}\".", - context - .get_dbfile() - .as_ref() - .map_or("<>", |p| p.to_str().unwrap()), + context.get_dbfile().display(), as_str(dest_pathNfilename), ); - if dc_copy_file( - context, - context.get_dbfile().unwrap(), - as_path(dest_pathNfilename), - ) { - context - .sql - .open(&context, &context.get_dbfile().unwrap(), 0); + if dc_copy_file(context, context.get_dbfile(), as_path(dest_pathNfilename)) { + context.sql.open(&context, &context.get_dbfile(), 0); closed = false; /* add all files as blobs to the database copy (this does not require the source to be locked, neigher the destination as it is used only here) */ /*for logging only*/ @@ -753,14 +735,14 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { } if ok_to_continue { let mut total_files_cnt = 0; - let dir = std::path::Path::new(as_str(context.get_blobdir())); - if let Ok(dir_handle) = std::fs::read_dir(dir) { + let dir = context.get_blobdir(); + if let Ok(dir_handle) = std::fs::read_dir(&dir) { total_files_cnt += dir_handle.filter(|r| r.is_ok()).count(); info!(context, "EXPORT: total_files_cnt={}", total_files_cnt); if total_files_cnt > 0 { // scan directory, pass 2: copy files - if let Ok(dir_handle) = std::fs::read_dir(dir) { + if let Ok(dir_handle) = std::fs::read_dir(&dir) { sql.prepare( "INSERT INTO backup_blobs (file_name, file_content) VALUES (?, ?);", move |mut stmt, _| { @@ -804,12 +786,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { continue; } else { info!(context, "EXPORTing filename={}", name); - let curr_pathNfilename = format!( - "{}/{}", - as_str(context.get_blobdir()), - name - ); - + let curr_pathNfilename = context.get_blobdir().join(entry.file_name()); if let Some(buf) = dc_read_file_safe(context, &curr_pathNfilename) { @@ -820,7 +797,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { error!( context, "Disk full? Cannot add file \"{}\" to backup.", - &curr_pathNfilename, + curr_pathNfilename.display(), ); /* this is not recoverable! writing to the sqlite database should work! */ ok_to_continue = false; @@ -839,7 +816,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { error!( context, "Backup: Cannot copy from blob-directory \"{}\".", - as_str(context.get_blobdir()), + context.get_blobdir().display(), ); } } else { @@ -863,16 +840,14 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool { error!( context, "Backup: Cannot get info for blob-directory \"{}\".", - as_str(context.get_blobdir()) + context.get_blobdir().display(), ); }; } } } if closed { - context - .sql - .open(&context, &context.get_dbfile().unwrap(), 0); + context.sql.open(&context, &context.get_dbfile(), 0); } if 0 != delete_dest_file { dc_delete_file(context, as_path(dest_pathNfilename)); diff --git a/src/dc_tools.rs b/src/dc_tools.rs index bf2009318..d86c565ab 100644 --- a/src/dc_tools.rs +++ b/src/dc_tools.rs @@ -798,11 +798,7 @@ pub fn dc_get_abs_path>( ) -> std::path::PathBuf { let p: &std::path::Path = path.as_ref(); if let Ok(p) = p.strip_prefix("$BLOBDIR") { - assert!( - context.has_blobdir(), - "Expected context to have blobdir to substitute $BLOBDIR", - ); - std::path::PathBuf::from(as_str(context.get_blobdir())).join(p) + context.get_blobdir().join(p) } else { p.into() } @@ -1001,13 +997,22 @@ pub unsafe fn dc_get_fine_pathNfilename( } pub fn dc_is_blobdir_path(context: &Context, path: impl AsRef) -> bool { - path.as_ref().starts_with(as_str(context.get_blobdir())) + context + .get_blobdir() + .to_str() + .map(|s| path.as_ref().starts_with(s)) + .unwrap_or_default() || path.as_ref().starts_with("$BLOBDIR") } fn dc_make_rel_path(context: &Context, path: &mut String) { - if path.starts_with(as_str(context.get_blobdir())) { - *path = path.replace("$BLOBDIR", as_str(context.get_blobdir())); + if context + .get_blobdir() + .to_str() + .map(|s| path.starts_with(s)) + .unwrap_or_default() + { + *path = path.replace("$BLOBDIR", context.get_blobdir().to_str().unwrap()); } } diff --git a/src/sql.rs b/src/sql.rs index c4b90854d..99a0d5d2d 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -791,7 +791,7 @@ fn open( let repl_from = sql .get_config(context, "backup_for") - .unwrap_or_else(|| to_string(context.get_blobdir())); + .unwrap_or_else(|| context.get_blobdir().to_string_lossy().into()); let repl_from = dc_ensure_no_slash_safe(&repl_from); sql.execute( @@ -990,7 +990,7 @@ pub fn housekeeping(context: &Context) { info!(context, "{} files in use.", files_in_use.len(),); /* go through directory and delete unused files */ - let p = std::path::Path::new(as_str(context.get_blobdir())); + let p = context.get_blobdir(); match std::fs::read_dir(p) { Ok(dir_handle) => { /* avoid deletion of files that are just created to build a message object */ @@ -1050,7 +1050,7 @@ pub fn housekeeping(context: &Context) { warn!( context, "Housekeeping: Cannot open {}. ({})", - as_str(context.get_blobdir()), + context.get_blobdir().display(), err ); } diff --git a/tests/stress.rs b/tests/stress.rs index de4d296f1..e3b2e84db 100644 --- a/tests/stress.rs +++ b/tests/stress.rs @@ -53,16 +53,13 @@ unsafe fn stress_functions(context: &Context) { 7i32 as libc::c_ulonglong ); - let abs_path: *mut libc::c_char = dc_mprintf( - b"%s/%s\x00" as *const u8 as *const libc::c_char, - context.get_blobdir(), - b"foobar\x00" as *const u8 as *const libc::c_char, - ); - assert!(dc_is_blobdir_path(context, as_str(abs_path))); + let abs_path = format!("{}/foobar", context.get_blobdir().to_string_lossy()); + + assert!(dc_is_blobdir_path(context, &abs_path)); assert!(dc_is_blobdir_path(context, "$BLOBDIR/fofo",)); assert!(!dc_is_blobdir_path(context, "/BLOBDIR/fofo",)); - assert!(dc_file_exist(context, as_path(abs_path))); - free(abs_path as *mut libc::c_void); + assert!(dc_file_exist(context, &abs_path)); + assert!(dc_copy_file(context, "$BLOBDIR/foobar", "$BLOBDIR/dada",)); assert_eq!(dc_get_filebytes(context, "$BLOBDIR/dada",), 7);