Remove dc_open() from the Rust API

This means the Context becomes a struct following the normal Rust
conventions where when it is created it is usable.  This will over
time allow removing a lot of runtime checks and simplify code.  Many
members will no longer need to be Options or similar.

The C API needs to remain compatible so hides the implementation of
this behind another struct which can be opened and closed.
This commit is contained in:
Floris Bruynooghe
2019-08-11 15:03:02 +02:00
parent 8a73f84003
commit b2031f5bcd
13 changed files with 2428 additions and 562 deletions

View File

@@ -1,3 +1,5 @@
use std::ffi::OsString;
use std::path::Path;
use std::sync::{Arc, Condvar, Mutex, RwLock};
use crate::chat::*;
@@ -7,6 +9,7 @@ use crate::dc_loginparam::*;
use crate::dc_move::*;
use crate::dc_receive_imf::*;
use crate::dc_tools::*;
use crate::error::*;
use crate::imap::*;
use crate::job::*;
use crate::job_thread::JobThread;
@@ -22,6 +25,15 @@ use std::ptr;
use std::path::PathBuf;
/// Callback function type for [Context].
///
/// @param context The context object as returned by dc_context_new().
/// @param event one of the @ref DC_EVENT constants
/// @param data1 depends on the event parameter
/// @param data2 depends on the event parameter
/// @return return 0 unless stated otherwise in the event parameter documentation
pub type ContextCallback = dyn Fn(&Context, Event, uintptr_t, uintptr_t) -> uintptr_t;
pub struct Context {
pub userdata: *mut libc::c_void,
pub dbfile: Arc<RwLock<Option<PathBuf>>>,
@@ -35,7 +47,7 @@ pub struct Context {
pub smtp: Arc<Mutex<Smtp>>,
pub smtp_state: Arc<(Mutex<SmtpState>, Condvar)>,
pub oauth2_critical: Arc<Mutex<()>>,
pub cb: Option<dc_callback_t>,
pub cb: Box<ContextCallback>,
pub os_name: Option<String>,
pub cmdline_sel_chat_id: Arc<RwLock<u32>>,
pub bob: Arc<RwLock<BobStatus>>,
@@ -55,6 +67,76 @@ pub struct RunningState {
}
impl Context {
pub fn new(cb: Box<ContextCallback>, os_name: String, dbfile: PathBuf) -> Result<Context> {
let mut blob_fname = OsString::new();
blob_fname.push(dbfile.file_name().unwrap_or_default());
blob_fname.push("-blobs");
let blobdir = dbfile.with_file_name(blob_fname);
if !blobdir.exists() {
std::fs::create_dir_all(&blobdir)?;
}
Context::with_blobdir(cb, os_name, dbfile, &blobdir)
}
pub fn with_blobdir(
cb: Box<ContextCallback>,
os_name: String,
dbfile: PathBuf,
blobdir: &Path,
) -> Result<Context> {
if !blobdir.is_dir() {
return Err(format_err!("Blobdir does not exist: {}", blobdir.display()));
}
let ctx = Context {
blobdir: Arc::new(RwLock::new(unsafe { blobdir.strdup() })),
dbfile: Arc::new(RwLock::new(Some(dbfile))),
inbox: Arc::new(RwLock::new({
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
)
})),
userdata: std::ptr::null_mut(),
cb,
os_name: Some(os_name),
running_state: Arc::new(RwLock::new(Default::default())),
sql: Sql::new(),
smtp: Arc::new(Mutex::new(Smtp::new())),
smtp_state: Arc::new((Mutex::new(Default::default()), Condvar::new())),
oauth2_critical: Arc::new(Mutex::new(())),
bob: Arc::new(RwLock::new(Default::default())),
last_smeared_timestamp: Arc::new(RwLock::new(0)),
cmdline_sel_chat_id: Arc::new(RwLock::new(0)),
sentbox_thread: Arc::new(RwLock::new(JobThread::new(
"SENTBOX",
"configured_sentbox_folder",
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
),
))),
mvbox_thread: Arc::new(RwLock::new(JobThread::new(
"MVBOX",
"configured_mvbox_folder",
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
),
))),
probe_imap_network: Arc::new(RwLock::new(false)),
perform_inbox_jobs_needed: Arc::new(RwLock::new(false)),
generating_key_mutex: Mutex::new(()),
};
ctx.sql.open(&ctx, &ctx.dbfile.read().unwrap().as_ref().unwrap(), 0)?;
Ok(ctx)
}
pub fn has_dbfile(&self) -> bool {
self.get_dbfile().is_some()
}
@@ -74,11 +156,7 @@ impl Context {
}
pub fn call_cb(&self, event: Event, data1: uintptr_t, data2: uintptr_t) -> uintptr_t {
if let Some(cb) = self.cb {
unsafe { cb(self, event, data1, data2) }
} else {
0
}
(*self.cb)(self, event, data1, data2)
}
}
@@ -115,60 +193,6 @@ pub struct SmtpState {
pub probe_network: bool,
}
// create/open/config/information
pub fn dc_context_new(
cb: Option<dc_callback_t>,
userdata: *mut libc::c_void,
os_name: Option<String>,
) -> Context {
Context {
blobdir: Arc::new(RwLock::new(std::ptr::null_mut())),
dbfile: Arc::new(RwLock::new(None)),
inbox: Arc::new(RwLock::new({
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
)
})),
userdata,
cb,
os_name,
running_state: Arc::new(RwLock::new(Default::default())),
sql: Sql::new(),
smtp: Arc::new(Mutex::new(Smtp::new())),
smtp_state: Arc::new((Mutex::new(Default::default()), Condvar::new())),
oauth2_critical: Arc::new(Mutex::new(())),
bob: Arc::new(RwLock::new(Default::default())),
last_smeared_timestamp: Arc::new(RwLock::new(0)),
cmdline_sel_chat_id: Arc::new(RwLock::new(0)),
sentbox_thread: Arc::new(RwLock::new(JobThread::new(
"SENTBOX",
"configured_sentbox_folder",
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
),
))),
mvbox_thread: Arc::new(RwLock::new(JobThread::new(
"MVBOX",
"configured_mvbox_folder",
Imap::new(
cb_get_config,
cb_set_config,
cb_precheck_imf,
cb_receive_imf,
),
))),
probe_imap_network: Arc::new(RwLock::new(false)),
perform_inbox_jobs_needed: Arc::new(RwLock::new(false)),
generating_key_mutex: Mutex::new(()),
}
}
unsafe fn cb_receive_imf(
context: &Context,
imf_raw_not_terminated: *const libc::c_char,
@@ -287,39 +311,10 @@ pub unsafe fn dc_close(context: &Context) {
*blobdir = ptr::null_mut();
}
pub unsafe fn dc_is_open(context: &Context) -> libc::c_int {
context.sql.is_open() as libc::c_int
}
pub unsafe fn dc_get_userdata(context: &mut Context) -> *mut libc::c_void {
context.userdata as *mut _
}
pub unsafe fn dc_open(context: &Context, dbfile: &str, blobdir: Option<&str>) -> bool {
let mut success = false;
if 0 != dc_is_open(context) {
return false;
}
*context.dbfile.write().unwrap() = Some(PathBuf::from(dbfile));
if blobdir.is_some() && !blobdir.unwrap().is_empty() {
let dir = dc_ensure_no_slash_safe(blobdir.unwrap()).strdup();
*context.blobdir.write().unwrap() = dir;
} else {
let dir = dbfile.to_string() + "-blobs";
dc_create_folder(context, &dir);
*context.blobdir.write().unwrap() = dir.strdup();
}
// Create/open sqlite database, this may already use the blobdir
let dbfile_path = std::path::Path::new(dbfile);
if context.sql.open(context, dbfile_path, 0) {
success = true
}
if !success {
dc_close(context);
}
success
}
pub unsafe fn dc_get_blobdir(context: &Context) -> *mut libc::c_char {
dc_strdup(*context.blobdir.clone().read().unwrap())
}
@@ -577,19 +572,70 @@ pub fn dc_is_mvbox(context: &Context, folder_name: impl AsRef<str>) -> bool {
mod tests {
use super::*;
use crate::test_utils::*;
#[test]
fn test_wrong_db() {
let tmp = tempfile::tempdir().unwrap();
let dbfile = tmp.path().join("db.sqlite");
std::fs::write(&dbfile, b"123").unwrap();
let res = Context::new(Box::new(|_, _, _, _| 0), "FakeOs".into(), dbfile);
assert!(res.is_err());
}
#[test]
fn test_blobdir_exists() {
let tmp = tempfile::tempdir().unwrap();
let dbfile = tmp.path().join("db.sqlite");
Context::new(Box::new(|_, _, _, _| 0), "FakeOS".into(), dbfile).unwrap();
let blobdir = tmp.path().join("db.sqlite-blobs");
assert!(blobdir.is_dir());
}
#[test]
fn test_wrong_blogdir() {
let tmp = tempfile::tempdir().unwrap();
let dbfile = tmp.path().join("db.sqlite");
let blobdir = tmp.path().join("db.sqlite-blobs");
std::fs::write(&blobdir, b"123").unwrap();
let res = Context::new(Box::new(|_, _, _, _| 0), "FakeOS".into(), dbfile);
assert!(res.is_err());
}
#[test]
fn test_sqlite_parent_not_exists() {
let tmp = tempfile::tempdir().unwrap();
let subdir = tmp.path().join("subdir");
let dbfile = subdir.join("db.sqlite");
let dbfile2 = dbfile.clone();
Context::new(Box::new(|_, _, _, _| 0), "FakeOS".into(), dbfile).unwrap();
assert!(subdir.is_dir());
assert!(dbfile2.is_file());
}
#[test]
fn test_with_blobdir_not_exists() {
let tmp = tempfile::tempdir().unwrap();
let dbfile = tmp.path().join("db.sqlite");
let blobdir = tmp.path().join("blobs");
let res =
Context::with_blobdir(Box::new(|_, _, _, _| 0), "FakeOS".into(), dbfile, &blobdir);
assert!(res.is_err());
}
#[test]
fn no_crashes_on_context_deref() {
let ctx = dc_context_new(None, std::ptr::null_mut(), Some("Test OS".into()));
std::mem::drop(ctx);
let t = dummy_context();
std::mem::drop(t.ctx);
}
#[test]
fn test_context_double_close() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
unsafe {
dc_close(&ctx);
dc_close(&ctx);
dc_close(&t.ctx);
dc_close(&t.ctx);
}
std::mem::drop(ctx);
std::mem::drop(t.ctx);
}
}

View File

@@ -74,7 +74,7 @@ pub unsafe fn dc_imex_has_backup(
let name = name.to_string_lossy();
if name.starts_with("delta-chat") && name.ends_with(".bak") {
let sql = Sql::new();
if sql.open(context, &path, 0x1) {
if sql.open(context, &path, 0x1).is_ok() {
let curr_backup_time =
sql.get_config_int(context, "backup_time")
.unwrap_or_default() as u64;
@@ -617,9 +617,10 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char
}
/* error already logged */
/* re-open copied database file */
if !context
if context
.sql
.open(&context, &context.get_dbfile().unwrap(), 0)
.is_err()
{
return 0;
}
@@ -710,7 +711,7 @@ unsafe fn import_backup(context: &Context, backup_to_import: *const libc::c_char
/* the FILE_PROGRESS macro calls the callback with the permille of files processed.
The macro avoids weird values of 0% or 100% while still working. */
// TODO should return bool /rtn
#[allow(non_snake_case)]
#[allow(non_snake_case, unused_must_use)]
unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> libc::c_int {
let mut current_block: u64;
let mut success: libc::c_int = 0;
@@ -757,7 +758,7 @@ unsafe fn export_backup(context: &Context, dir: *const libc::c_char) -> libc::c_
/* 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*/
let sql = Sql::new();
if sql.open(context, as_path(dest_pathNfilename), 0) {
if sql.open(context, as_path(dest_pathNfilename), 0).is_ok() {
if !sql.table_exists("backup_blobs") {
if sql::execute(
context,
@@ -1159,7 +1160,7 @@ mod tests {
#[test]
fn test_render_setup_file() {
let t = test_context(Some(logging_cb));
let t = test_context(Some(Box::new(logging_cb)));
configure_alice_keypair(&t.ctx);
let msg = dc_render_setup_file(&t.ctx, "hello").unwrap();
@@ -1177,14 +1178,9 @@ mod tests {
assert!(msg.contains("-----END PGP MESSAGE-----\n"));
}
unsafe extern "C" fn ac_setup_msg_cb(
ctx: &Context,
evt: Event,
d1: uintptr_t,
d2: uintptr_t,
) -> uintptr_t {
fn ac_setup_msg_cb(ctx: &Context, evt: Event, d1: uintptr_t, d2: uintptr_t) -> uintptr_t {
if evt == Event::GET_STRING && d1 == StockMessage::AcSetupMsgBody.to_usize().unwrap() {
"hello\r\nthere".strdup() as usize
unsafe { "hello\r\nthere".strdup() as usize }
} else {
logging_cb(ctx, evt, d1, d2)
}
@@ -1192,7 +1188,7 @@ mod tests {
#[test]
fn test_render_setup_file_newline_replace() {
let t = test_context(Some(ac_setup_msg_cb));
let t = test_context(Some(Box::new(ac_setup_msg_cb)));
configure_alice_keypair(&t.ctx);
let msg = dc_render_setup_file(&t.ctx, "pw").unwrap();
println!("{}", &msg);

View File

@@ -8,14 +8,13 @@ use std::{fmt, fs, ptr};
use chrono::{Local, TimeZone};
use mmime::mailimf_types::*;
use rand::{thread_rng, Rng};
use itertools::max;
use crate::context::Context;
use crate::error::Error;
use crate::types::*;
use crate::x::*;
use itertools::max;
/* Some tools and enhancements to the used libraries, there should be
no references to Context and other "larger" classes here. */
/* ** library-private **********************************************************/
@@ -1228,7 +1227,7 @@ impl CStringExt for CString {}
/// 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 {
pub trait Strdup {
/// Allocate a new raw C `*char` version of this string.
///
/// This allocates a new raw C string which must be freed using
@@ -1245,13 +1244,26 @@ pub trait StrExt {
unsafe fn strdup(&self) -> *mut libc::c_char;
}
impl<T: AsRef<str>> StrExt for T {
impl<T: AsRef<str>> Strdup for T {
unsafe fn strdup(&self) -> *mut libc::c_char {
let tmp = CString::yolo(self.as_ref());
dc_strdup(tmp.as_ptr())
}
}
impl Strdup for CStr {
unsafe fn strdup(&self) -> *mut libc::c_char {
dc_strdup(self.as_ptr())
}
}
impl Strdup for std::path::Path {
unsafe fn strdup(&self) -> *mut libc::c_char {
let tmp = self.to_c_string().unwrap();
dc_strdup(tmp.as_ptr())
}
}
pub fn to_string(s: *const libc::c_char) -> String {
if s.is_null() {
return "".into();

View File

@@ -38,16 +38,13 @@ impl Sql {
info!(context, 0, "Database closed.");
}
// return true on success, false on failure
pub fn open(&self, context: &Context, dbfile: &std::path::Path, flags: libc::c_int) -> bool {
match open(context, self, dbfile, flags) {
Ok(_) => true,
Err(Error::SqlAlreadyOpen) => false,
Err(_) => {
self.close(context);
false
}
}
pub fn open(
&self,
context: &Context,
dbfile: &std::path::Path,
flags: libc::c_int,
) -> Result<()> {
open(context, self, dbfile, flags)
}
pub fn execute<P>(&self, sql: &str, params: P) -> Result<usize>
@@ -227,7 +224,10 @@ impl Sql {
match res {
Ok(_) => Ok(()),
Err(err) => {
error!(context, 0, "set_config(): Cannot change value. {:?}", &err);
error!(
context,
0, "set_config(): Cannot change value for {}: {:?}", key, &err
);
Err(err.into())
}
}

View File

@@ -232,10 +232,7 @@ mod tests {
use super::*;
use crate::test_utils::*;
use std::ffi::CString;
use crate::constants::DC_CONTACT_ID_SELF;
use crate::context::dc_context_new;
use crate::types::uintptr_t;
use num_traits::ToPrimitive;
@@ -253,19 +250,18 @@ mod tests {
#[test]
fn test_stock_str() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
assert_eq!(ctx.stock_str(StockMessage::NoMessages), "No messages.");
let t = dummy_context();
assert_eq!(t.ctx.stock_str(StockMessage::NoMessages), "No messages.");
}
unsafe extern "C" fn test_stock_str_no_fallback_cb(
fn test_stock_str_no_fallback_cb(
_ctx: &Context,
evt: Event,
d1: uintptr_t,
_d2: uintptr_t,
) -> uintptr_t {
if evt == Event::GET_STRING && d1 == StockMessage::NoMessages.to_usize().unwrap() {
let tmp = CString::new("Hello there").unwrap();
dc_strdup(tmp.as_ptr()) as usize
unsafe { "Hello there".strdup() as usize }
} else {
0
}
@@ -273,16 +269,16 @@ mod tests {
#[test]
fn test_stock_str_no_fallback() {
let t = test_context(Some(test_stock_str_no_fallback_cb));
let t = test_context(Some(Box::new(test_stock_str_no_fallback_cb)));
assert_eq!(t.ctx.stock_str(StockMessage::NoMessages), "Hello there");
}
#[test]
fn test_stock_string_repl_str() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
// uses %1$s substitution
assert_eq!(
ctx.stock_string_repl_str(StockMessage::Member, "42"),
t.ctx.stock_string_repl_str(StockMessage::Member, "42"),
"42 member(s)"
);
// We have no string using %1$d to test...
@@ -290,36 +286,38 @@ mod tests {
#[test]
fn test_stock_string_repl_int() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
assert_eq!(
ctx.stock_string_repl_int(StockMessage::Member, 42),
t.ctx.stock_string_repl_int(StockMessage::Member, 42),
"42 member(s)"
);
}
#[test]
fn test_stock_string_repl_str2() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
assert_eq!(
ctx.stock_string_repl_str2(StockMessage::ServerResponse, "foo", "bar"),
t.ctx
.stock_string_repl_str2(StockMessage::ServerResponse, "foo", "bar"),
"Response from foo: bar"
);
}
#[test]
fn test_stock_system_msg_simple() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
assert_eq!(
ctx.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", 0),
t.ctx
.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", 0),
"Location streaming enabled."
)
}
#[test]
fn test_stock_system_msg_add_member_by_me() {
let ctx = dc_context_new(None, std::ptr::null_mut(), None);
let t = dummy_context();
assert_eq!(
ctx.stock_system_msg(
t.ctx.stock_system_msg(
StockMessage::MsgAddMember,
"alice@example.com",
"",

View File

@@ -9,9 +9,8 @@ use tempfile::{tempdir, TempDir};
use crate::config::Config;
use crate::constants::{Event, KeyType};
use crate::context::{dc_context_new, dc_open, Context};
use crate::context::{Context, ContextCallback};
use crate::key;
use crate::types::dc_callback_t;
/// A Context and temporary directory.
///
@@ -28,18 +27,15 @@ pub struct TestContext {
/// "db.sqlite" in the [TestContext.dir] directory.
///
/// [Context]: crate::context::Context
pub fn test_context(cb: Option<dc_callback_t>) -> TestContext {
unsafe {
let mut ctx = dc_context_new(cb, std::ptr::null_mut(), None);
let dir = tempdir().unwrap();
let dbfile = dir.path().join("db.sqlite");
assert!(
dc_open(&mut ctx, dbfile.to_str().unwrap(), None),
"Failed to open {}",
dbfile.display(),
);
TestContext { ctx: ctx, dir: dir }
}
pub fn test_context(callback: Option<Box<ContextCallback>>) -> TestContext {
let dir = tempdir().unwrap();
let dbfile = dir.path().join("db.sqlite");
let cb: Box<ContextCallback> = match callback {
Some(cb) => cb,
None => Box::new(|_, _, _, _| 0),
};
let ctx = Context::new(cb, "FakeOs".into(), dbfile).unwrap();
TestContext { ctx: ctx, dir: dir }
}
/// Return a dummy [TestContext].
@@ -51,13 +47,8 @@ pub fn dummy_context() -> TestContext {
test_context(None)
}
pub unsafe extern "C" fn logging_cb(
_ctx: &Context,
evt: Event,
_d1: uintptr_t,
d2: uintptr_t,
) -> uintptr_t {
let to_str = |x| CStr::from_ptr(x as *const libc::c_char).to_str().unwrap();
pub fn logging_cb(_ctx: &Context, evt: Event, _d1: uintptr_t, d2: uintptr_t) -> uintptr_t {
let to_str = unsafe { |x| CStr::from_ptr(x as *const libc::c_char).to_str().unwrap() };
match evt {
Event::INFO => println!("I: {}", to_str(d2)),
Event::WARNING => println!("W: {}", to_str(d2)),

View File

@@ -1,21 +1,9 @@
#![allow(non_camel_case_types)]
use crate::constants::Event;
use crate::context::Context;
pub use mmime::clist::*;
pub use rusqlite::ffi::*;
/// Callback function that should be given to dc_context_new().
///
/// @memberof Context
/// @param context The context object as returned by dc_context_new().
/// @param event one of the @ref DC_EVENT constants
/// @param data1 depends on the event parameter
/// @param data2 depends on the event parameter
/// @return return 0 unless stated otherwise in the event parameter documentation
pub type dc_callback_t =
unsafe extern "C" fn(_: &Context, _: Event, _: uintptr_t, _: uintptr_t) -> uintptr_t;
pub type dc_receive_imf_t = unsafe fn(
_: &Context,
_: *const libc::c_char,