diff --git a/lib/src/replica.rs b/lib/src/replica.rs index a1d488ede..11fd26186 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -94,6 +94,34 @@ where } } +/// Utility function to allow using `?` notation to return an error value in the constructor. +fn wrap_constructor(f: F, error_out: *mut TCString, err_value: T) -> T +where + F: FnOnce() -> anyhow::Result, +{ + if !error_out.is_null() { + // SAFETY: + // - error_out is not NULL (just checked) + // - properly aligned and valid (promised by caller) + unsafe { *error_out = TCString::default() }; + } + + match f() { + Ok(v) => v, + Err(e) => { + if !error_out.is_null() { + // SAFETY: + // - error_out is not NULL (just checked) + // - properly aligned and valid (promised by caller) + unsafe { + TCString::val_to_arg_out(err_to_ruststring(e), error_out); + } + } + err_value + } + } +} + /// Create a new TCReplica with an in-memory database. The contents of the database will be /// lost when it is freed with tc_replica_free. #[no_mangle] @@ -114,40 +142,24 @@ pub unsafe extern "C" fn tc_replica_new_on_disk( path: TCString, error_out: *mut TCString, ) -> *mut TCReplica { - if !error_out.is_null() { - // SAFETY: - // - error_out is not NULL (just checked) - // - properly aligned and valid (promised by caller) - unsafe { *error_out = TCString::default() }; - } - - // SAFETY: - // - path is valid (promised by caller) - // - caller will not use path after this call (convention) - let path = unsafe { TCString::val_from_arg(path) }; - let storage_res = StorageConfig::OnDisk { - taskdb_dir: path.to_path_buf(), - } - .into_storage(); - - let storage = match storage_res { - Ok(storage) => storage, - Err(e) => { - if !error_out.is_null() { - unsafe { - // SAFETY: - // - error_out is not NULL (just checked) - // - properly aligned and valid (promised by caller) - TCString::val_to_arg_out(err_to_ruststring(e), error_out); - } + wrap_constructor( + || { + // SAFETY: + // - path is valid (promised by caller) + // - caller will not use path after this call (convention) + let mut path = unsafe { TCString::val_from_arg(path) }; + let storage = StorageConfig::OnDisk { + taskdb_dir: path.to_path_buf()?, } - return std::ptr::null_mut(); - } - }; + .into_storage()?; - // SAFETY: - // - caller promises to free this value - unsafe { TCReplica::from(Replica::new(storage)).return_ptr() } + // SAFETY: + // - caller promises to free this value + Ok(unsafe { TCReplica::from(Replica::new(storage)).return_ptr() }) + }, + error_out, + std::ptr::null_mut(), + ) } /// Get a list of all tasks in the replica. diff --git a/lib/src/server.rs b/lib/src/server.rs index 711fcacf5..b7247bdb6 100644 --- a/lib/src/server.rs +++ b/lib/src/server.rs @@ -70,9 +70,9 @@ pub unsafe extern "C" fn tc_server_new_local( // SAFETY: // - server_dir is valid (promised by caller) // - caller will not use server_dir after this call (convention) - let server_dir = unsafe { TCString::val_from_arg(server_dir) }; + let mut server_dir = unsafe { TCString::val_from_arg(server_dir) }; let server_config = ServerConfig::Local { - server_dir: server_dir.to_path_buf(), + server_dir: server_dir.to_path_buf()?, }; let server = server_config.into_server()?; // SAFETY: caller promises to free this server. diff --git a/lib/src/string.rs b/lib/src/string.rs index 3767f7bb2..2791aadcc 100644 --- a/lib/src/string.rs +++ b/lib/src/string.rs @@ -1,8 +1,7 @@ use crate::traits::*; use crate::util::{string_into_raw_parts, vec_into_raw_parts}; -use std::ffi::{CStr, CString, OsStr}; +use std::ffi::{CStr, CString, OsStr, OsString}; use std::os::raw::c_char; -use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; /// TCString supports passing strings into and out of the TaskChampion API. @@ -293,10 +292,21 @@ impl<'a> RustString<'a> { } } - pub(crate) fn to_path_buf(&self) -> PathBuf { - // TODO: this is UNIX-specific. - let path: &OsStr = OsStr::from_bytes(self.as_bytes()); - path.to_os_string().into() + pub(crate) fn to_path_buf(&mut self) -> Result { + #[cfg(unix)] + let path: OsString = { + // on UNIX, we can use the bytes directly, without requiring that they + // be valid UTF-8. + use std::os::unix::ffi::OsStrExt; + OsStr::from_bytes(self.as_bytes()).to_os_string() + }; + #[cfg(windows)] + let path: OsString = { + // on Windows, we assume the filename is valid Unicode, so it can be + // represented as UTF-8. + OsString::from(self.as_str()?.to_string()) + }; + Ok(path.into()) } }