Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-05-02 21:06:31 +03:00
Code Issues Packages Projects Releases Wiki Activity

refactor(context): remove last unsafe bits from the context struct

Browse Source
This commit is contained in:
dignifiedquire
2019-09-15 12:46:03 +02:00
committed by Floris Bruynooghe
parent a3f64d4e95
commit de1e3e1d4f
7 changed files with 81 additions and 139 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
deltachat-ffi/src/lib.rs
View File

@@ -52,6 +52,9 @@ pub struct ContextWrapper {
inner: RwLock<Option<context::Context>>, inner: RwLock<Option<context::Context>>,
} }
unsafe impl Send for ContextWrapper {}
unsafe impl Sync for ContextWrapper {}
/// Callback function that should be given to [dc_context_new]. /// Callback function that should be given to [dc_context_new].
/// ///
/// @memberof [dc_context_t] /// @memberof [dc_context_t]
@@ -169,13 +172,13 @@ pub unsafe extern "C" fn dc_open(
eprintln!("ignoring careless call to dc_open()"); eprintln!("ignoring careless call to dc_open()");
return 0; return 0;
} }
let rust_cb = move |_ctx: &Context, evt: Event, d0: uintptr_t, d1: uintptr_t| { let ffi_context = &*context;
let ffi_context = &*context;
match ffi_context.cb { 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), Some(ffi_cb) => ffi_cb(ffi_context, evt, d0, d1),
None => 0, None => 0,
} };
};
let ffi_context = &mut *context; let ffi_context = &mut *context;
let ctx = if blobdir.is_null() { let ctx = if blobdir.is_null() {
Context::new( 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; let ffi_context = &*context;
ffi_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()) .unwrap_or_else(|_| "".strdup())
} }
@@ -1413,7 +1416,7 @@ pub unsafe extern "C" fn dc_imex(
} }
let ffi_context = &*context; let ffi_context = &*context;
ffi_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(); .ok();
} }
@@ -1428,7 +1431,7 @@ pub unsafe extern "C" fn dc_imex_has_backup(
} }
let ffi_context = &*context; let ffi_context = &*context;
ffi_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()) .unwrap_or_else(|_| ptr::null_mut())
} }

8
examples/repl/cmdline.rs
View File

@@ -491,17 +491,17 @@ pub unsafe fn dc_cmdline(context: &Context, line: &str) -> Result<(), failure::E
} }
} }
"export-backup" => { "export-backup" => {
dc_imex(context, 11, context.get_blobdir(), ptr::null()); dc_imex(context, 11, Some(context.get_blobdir()), ptr::null());
} }
"import-backup" => { "import-backup" => {
ensure!(!arg1.is_empty(), "Argument <backup-file> missing."); ensure!(!arg1.is_empty(), "Argument <backup-file> missing.");
dc_imex(context, 12, arg1_c, ptr::null()); dc_imex(context, 12, Some(arg1), ptr::null());
} }
"export-keys" => { "export-keys" => {
dc_imex(context, 1, context.get_blobdir(), ptr::null()); dc_imex(context, 1, Some(context.get_blobdir()), ptr::null());
} }
"import-keys" => { "import-keys" => {
dc_imex(context, 2, context.get_blobdir(), ptr::null()); dc_imex(context, 2, Some(context.get_blobdir()), ptr::null());
} }
"export-setup" => { "export-setup" => {
let setup_code = dc_create_setup_code(context); let setup_code = dc_create_setup_code(context);

80
src/context.rs
View File

@@ -35,12 +35,11 @@ use crate::x::*;
/// ///
/// This callback must return 0 unless stated otherwise in the event /// This callback must return 0 unless stated otherwise in the event
/// description at [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 struct Context {
pub userdata: *mut libc::c_void, pub dbfile: Arc<RwLock<PathBuf>>,
pub dbfile: Arc<RwLock<Option<PathBuf>>>, pub blobdir: Arc<RwLock<PathBuf>>,
pub blobdir: Arc<RwLock<*mut libc::c_char>>,
pub sql: Sql, pub sql: Sql,
pub inbox: Arc<RwLock<Imap>>, pub inbox: Arc<RwLock<Imap>>,
pub perform_inbox_jobs_needed: Arc<RwLock<bool>>, pub perform_inbox_jobs_needed: Arc<RwLock<bool>>,
@@ -60,9 +59,6 @@ pub struct Context {
pub generating_key_mutex: Mutex<()>, pub generating_key_mutex: Mutex<()>,
} }
unsafe impl std::marker::Send for Context {}
unsafe impl std::marker::Sync for Context {}
#[derive(Debug, PartialEq, Eq)] #[derive(Debug, PartialEq, Eq)]
pub struct RunningState { pub struct RunningState {
pub ongoing_running: bool, pub ongoing_running: bool,
@@ -92,10 +88,9 @@ impl Context {
"Blobdir does not exist: {}", "Blobdir does not exist: {}",
blobdir.display() blobdir.display()
); );
let blobdir_c = blobdir.to_c_string()?;
let ctx = Context { let ctx = Context {
blobdir: Arc::new(RwLock::new(unsafe { dc_strdup(blobdir_c.as_ptr()) })), blobdir: Arc::new(RwLock::new(blobdir)),
dbfile: Arc::new(RwLock::new(Some(dbfile))), dbfile: Arc::new(RwLock::new(dbfile)),
inbox: Arc::new(RwLock::new({ inbox: Arc::new(RwLock::new({
Imap::new( Imap::new(
cb_get_config, cb_get_config,
@@ -104,7 +99,6 @@ impl Context {
cb_receive_imf, cb_receive_imf,
) )
})), })),
userdata: std::ptr::null_mut(),
cb, cb,
os_name: Some(os_name), os_name: Some(os_name),
running_state: Arc::new(RwLock::new(Default::default())), running_state: Arc::new(RwLock::new(Default::default())),
@@ -139,31 +133,18 @@ impl Context {
perform_inbox_jobs_needed: Arc::new(RwLock::new(false)), perform_inbox_jobs_needed: Arc::new(RwLock::new(false)),
generating_key_mutex: Mutex::new(()), generating_key_mutex: Mutex::new(()),
}; };
if !ctx if !ctx.sql.open(&ctx, &ctx.dbfile.read().unwrap(), 0) {
.sql
.open(&ctx, &ctx.dbfile.read().unwrap().as_ref().unwrap(), 0)
{
return Err(format_err!("Failed opening sqlite database")); return Err(format_err!("Failed opening sqlite database"));
} }
Ok(ctx) Ok(ctx)
} }
pub fn has_dbfile(&self) -> bool { pub fn get_dbfile(&self) -> PathBuf {
self.get_dbfile().is_some() self.dbfile.clone().read().unwrap().clone()
} }
pub fn has_blobdir(&self) -> bool { pub fn get_blobdir(&self) -> PathBuf {
!self.get_blobdir().is_null() self.blobdir.clone().read().unwrap().clone()
}
pub fn get_dbfile(&self) -> Option<PathBuf> {
(*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 call_cb(&self, event: Event, data1: uintptr_t, data2: uintptr_t) -> uintptr_t { 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 { impl Drop for Context {
fn drop(&mut self) { fn drop(&mut self) {
unsafe { info!(self, "disconnecting INBOX-watch",);
info!(self, "disconnecting INBOX-watch",); self.inbox.read().unwrap().disconnect(self);
self.inbox.read().unwrap().disconnect(self); info!(self, "disconnecting sentbox-thread",);
info!(self, "disconnecting sentbox-thread",); self.sentbox_thread.read().unwrap().imap.disconnect(self);
self.sentbox_thread.read().unwrap().imap.disconnect(self); info!(self, "disconnecting mvbox-thread",);
info!(self, "disconnecting mvbox-thread",); self.mvbox_thread.read().unwrap().imap.disconnect(self);
self.mvbox_thread.read().unwrap().imap.disconnect(self); info!(self, "disconnecting SMTP");
info!(self, "disconnecting SMTP"); self.smtp.clone().lock().unwrap().disconnect();
self.smtp.clone().lock().unwrap().disconnect(); self.sql.close(self);
self.sql.close(self);
let blobdir = self.blobdir.write().unwrap();
free(*blobdir as *mut libc::c_void);
}
} }
} }
@@ -297,14 +274,6 @@ fn cb_get_config(context: &Context, key: &str) -> Option<String> {
context.sql.get_config(context, key) 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 * INI-handling, Information
******************************************************************************/ ******************************************************************************/
@@ -421,16 +390,9 @@ pub unsafe fn dc_get_info(context: &Context) -> *mut libc::c_char {
real_msgs, real_msgs,
deaddrop_msgs, deaddrop_msgs,
contacts, contacts,
context context.get_dbfile().to_str().unwrap(),
.get_dbfile()
.as_ref()
.map_or(unset, |p| p.to_str().unwrap()),
dbversion, dbversion,
if context.has_blobdir() { context.get_blobdir().to_str().unwrap(),
as_str(context.get_blobdir())
} else {
unset
},
displayname.unwrap_or_else(|| unset.into()), displayname.unwrap_or_else(|| unset.into()),
is_configured, is_configured,
l, l,

73
src/dc_imex.rs
View File

@@ -1,4 +1,5 @@
use std::ffi::CString; use std::ffi::CString;
use std::path::Path;
use std::ptr; use std::ptr;
use mmime::mailmime_content::*; use mmime::mailmime_content::*;
@@ -32,13 +33,13 @@ use crate::x::*;
pub fn dc_imex( pub fn dc_imex(
context: &Context, context: &Context,
what: libc::c_int, what: libc::c_int,
param1: *const libc::c_char, param1: Option<impl AsRef<Path>>,
param2: *const libc::c_char, param2: *const libc::c_char,
) { ) {
let mut param = Params::new(); let mut param = Params::new();
param.set_int(Param::Cmd, what as i32); param.set_int(Param::Cmd, what as i32);
if !param1.is_null() { if let Some(param1) = param1 {
param.set(Param::Arg, as_str(param1)); param.set(Param::Arg, param1.as_ref().to_string_lossy());
} }
if !param2.is_null() { if !param2.is_null() {
param.set(Param::Arg2, as_str(param2)); 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. /// Returns the filename of the backup if found, nullptr otherwise.
pub unsafe fn dc_imex_has_backup( pub unsafe fn dc_imex_has_backup(
context: &Context, context: &Context,
dir_name: *const libc::c_char, dir_name: impl AsRef<Path>,
) -> *mut libc::c_char { ) -> *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); let dir_iter = std::fs::read_dir(dir_name);
if dir_iter.is_err() { if dir_iter.is_err() {
info!( info!(
@@ -567,10 +568,7 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char
context, context,
"Import \"{}\" to \"{}\".", "Import \"{}\" to \"{}\".",
as_str(backup_to_import), as_str(backup_to_import),
context context.get_dbfile().display()
.get_dbfile()
.as_ref()
.map_or("<<None>>", |p| p.to_str().unwrap())
); );
if dc_is_configured(context) { if dc_is_configured(context) {
@@ -578,8 +576,8 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char
return false; return false;
} }
&context.sql.close(&context); &context.sql.close(&context);
dc_delete_file(context, context.get_dbfile().unwrap()); dc_delete_file(context, context.get_dbfile());
if dc_file_exist(context, context.get_dbfile().unwrap()) { if dc_file_exist(context, context.get_dbfile()) {
error!( error!(
context, context,
"Cannot import backups: Cannot delete the old file.", "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; return false;
} }
if !dc_copy_file( if !dc_copy_file(context, as_path(backup_to_import), context.get_dbfile()) {
context,
as_path(backup_to_import),
context.get_dbfile().unwrap(),
) {
return false; return false;
} }
/* error already logged */ /* error already logged */
/* re-open copied database file */ /* re-open copied database file */
if !context if !context.sql.open(&context, &context.get_dbfile(), 0) {
.sql
.open(&context, &context.get_dbfile().unwrap(), 0)
{
return false; return false;
} }
@@ -651,14 +642,14 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char
continue; 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) { if dc_write_file_safe(context, &pathNfilename, &file_blob) {
continue; continue;
} }
error!( error!(
context, context,
"Storage full? Cannot write file {} with {} bytes.", "Storage full? Cannot write file {} with {} bytes.",
&pathNfilename, pathNfilename.display(),
file_blob.len(), file_blob.len(),
); );
// otherwise the user may believe the stuff is imported correctly, but there are files missing ... // 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!( info!(
context, context,
"Backup \"{}\" to \"{}\".", "Backup \"{}\" to \"{}\".",
context context.get_dbfile().display(),
.get_dbfile()
.as_ref()
.map_or("<<None>>", |p| p.to_str().unwrap()),
as_str(dest_pathNfilename), as_str(dest_pathNfilename),
); );
if dc_copy_file( if dc_copy_file(context, context.get_dbfile(), as_path(dest_pathNfilename)) {
context, context.sql.open(&context, &context.get_dbfile(), 0);
context.get_dbfile().unwrap(),
as_path(dest_pathNfilename),
) {
context
.sql
.open(&context, &context.get_dbfile().unwrap(), 0);
closed = false; 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) */ /* 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*/ /*for logging only*/
@@ -753,14 +735,14 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool {
} }
if ok_to_continue { if ok_to_continue {
let mut total_files_cnt = 0; let mut total_files_cnt = 0;
let dir = std::path::Path::new(as_str(context.get_blobdir())); let dir = context.get_blobdir();
if let Ok(dir_handle) = std::fs::read_dir(dir) { if let Ok(dir_handle) = std::fs::read_dir(&dir) {
total_files_cnt += dir_handle.filter(|r| r.is_ok()).count(); total_files_cnt += dir_handle.filter(|r| r.is_ok()).count();
info!(context, "EXPORT: total_files_cnt={}", total_files_cnt); info!(context, "EXPORT: total_files_cnt={}", total_files_cnt);
if total_files_cnt > 0 { if total_files_cnt > 0 {
// scan directory, pass 2: copy files // 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( sql.prepare(
"INSERT INTO backup_blobs (file_name, file_content) VALUES (?, ?);", "INSERT INTO backup_blobs (file_name, file_content) VALUES (?, ?);",
move |mut stmt, _| { move |mut stmt, _| {
@@ -804,12 +786,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool {
continue; continue;
} else { } else {
info!(context, "EXPORTing filename={}", name); info!(context, "EXPORTing filename={}", name);
let curr_pathNfilename = format!( let curr_pathNfilename = context.get_blobdir().join(entry.file_name());
"{}/{}",
as_str(context.get_blobdir()),
name
);
if let Some(buf) = if let Some(buf) =
dc_read_file_safe(context, &curr_pathNfilename) dc_read_file_safe(context, &curr_pathNfilename)
{ {
@@ -820,7 +797,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool {
error!( error!(
context, context,
"Disk full? Cannot add file \"{}\" to backup.", "Disk full? Cannot add file \"{}\" to backup.",
&curr_pathNfilename, curr_pathNfilename.display(),
); );
/* this is not recoverable! writing to the sqlite database should work! */ /* this is not recoverable! writing to the sqlite database should work! */
ok_to_continue = false; ok_to_continue = false;
@@ -839,7 +816,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool {
error!( error!(
context, context,
"Backup: Cannot copy from blob-directory \"{}\".", "Backup: Cannot copy from blob-directory \"{}\".",
as_str(context.get_blobdir()), context.get_blobdir().display(),
); );
} }
} else { } else {
@@ -863,16 +840,14 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> bool {
error!( error!(
context, context,
"Backup: Cannot get info for blob-directory \"{}\".", "Backup: Cannot get info for blob-directory \"{}\".",
as_str(context.get_blobdir()) context.get_blobdir().display(),
); );
}; };
} }
} }
} }
if closed { if closed {
context context.sql.open(&context, &context.get_dbfile(), 0);
.sql
.open(&context, &context.get_dbfile().unwrap(), 0);
} }
if 0 != delete_dest_file { if 0 != delete_dest_file {
dc_delete_file(context, as_path(dest_pathNfilename)); dc_delete_file(context, as_path(dest_pathNfilename));

21
src/dc_tools.rs
View File

@@ -798,11 +798,7 @@ pub fn dc_get_abs_path<P: AsRef<std::path::Path>>(
) -> std::path::PathBuf { ) -> std::path::PathBuf {
let p: &std::path::Path = path.as_ref(); let p: &std::path::Path = path.as_ref();
if let Ok(p) = p.strip_prefix("$BLOBDIR") { if let Ok(p) = p.strip_prefix("$BLOBDIR") {
assert!( context.get_blobdir().join(p)
context.has_blobdir(),
"Expected context to have blobdir to substitute $BLOBDIR",
);
std::path::PathBuf::from(as_str(context.get_blobdir())).join(p)
} else { } else {
p.into() p.into()
} }
@@ -1001,13 +997,22 @@ pub unsafe fn dc_get_fine_pathNfilename(
} }
pub fn dc_is_blobdir_path(context: &Context, path: impl AsRef<str>) -> bool { pub fn dc_is_blobdir_path(context: &Context, path: impl AsRef<str>) -> 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") || path.as_ref().starts_with("$BLOBDIR")
} }
fn dc_make_rel_path(context: &Context, path: &mut String) { fn dc_make_rel_path(context: &Context, path: &mut String) {
if path.starts_with(as_str(context.get_blobdir())) { if context
*path = path.replace("$BLOBDIR", as_str(context.get_blobdir())); .get_blobdir()
.to_str()
.map(|s| path.starts_with(s))
.unwrap_or_default()
{
*path = path.replace("$BLOBDIR", context.get_blobdir().to_str().unwrap());
} }
} }

6
src/sql.rs
View File

@@ -791,7 +791,7 @@ fn open(
let repl_from = sql let repl_from = sql
.get_config(context, "backup_for") .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); let repl_from = dc_ensure_no_slash_safe(&repl_from);
sql.execute( sql.execute(
@@ -990,7 +990,7 @@ pub fn housekeeping(context: &Context) {
info!(context, "{} files in use.", files_in_use.len(),); info!(context, "{} files in use.", files_in_use.len(),);
/* go through directory and delete unused files */ /* 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) { match std::fs::read_dir(p) {
Ok(dir_handle) => { Ok(dir_handle) => {
/* avoid deletion of files that are just created to build a message object */ /* avoid deletion of files that are just created to build a message object */
@@ -1050,7 +1050,7 @@ pub fn housekeeping(context: &Context) {
warn!( warn!(
context, context,
"Housekeeping: Cannot open {}. ({})", "Housekeeping: Cannot open {}. ({})",
as_str(context.get_blobdir()), context.get_blobdir().display(),
err err
); );
} }

13
tests/stress.rs
View File

@@ -53,16 +53,13 @@ unsafe fn stress_functions(context: &Context) {
7i32 as libc::c_ulonglong 7i32 as libc::c_ulonglong
); );
let abs_path: *mut libc::c_char = dc_mprintf( let abs_path = format!("{}/foobar", context.get_blobdir().to_string_lossy());
b"%s/%s\x00" as *const u8 as *const libc::c_char,
context.get_blobdir(), assert!(dc_is_blobdir_path(context, &abs_path));
b"foobar\x00" as *const u8 as *const libc::c_char,
);
assert!(dc_is_blobdir_path(context, as_str(abs_path)));
assert!(dc_is_blobdir_path(context, "$BLOBDIR/fofo",)); assert!(dc_is_blobdir_path(context, "$BLOBDIR/fofo",));
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))); assert!(dc_file_exist(context, &abs_path));
free(abs_path as *mut libc::c_void);
assert!(dc_copy_file(context, "$BLOBDIR/foobar", "$BLOBDIR/dada",)); assert!(dc_copy_file(context, "$BLOBDIR/foobar", "$BLOBDIR/dada",));
assert_eq!(dc_get_filebytes(context, "$BLOBDIR/dada",), 7); assert_eq!(dc_get_filebytes(context, "$BLOBDIR/dada",), 7);