diff --git a/lib/src/arrays.rs b/lib/src/arrays.rs index e94ead43b..3c266cb3c 100644 --- a/lib/src/arrays.rs +++ b/lib/src/arrays.rs @@ -1,4 +1,5 @@ use crate::string::TCString; +use crate::traits::*; use std::ptr::NonNull; // TODO: generalize to TCStrings? @@ -88,10 +89,8 @@ pub extern "C" fn tc_tags_free<'a>(tctags: *mut TCTags) { // drop each contained string for tcstring in vec.drain(..) { - // SAFETY: - // - the pointer is not NULL (as created by TCString::new) - // - C does not expect the string's lifetime to exceed this function - drop(unsafe { TCString::from_arg(tcstring.as_ptr()) }); + // SAFETY: see TCString docstring + drop(unsafe { TCString::take_from_arg(tcstring.as_ptr()) }); } drop(vec); diff --git a/lib/src/lib.rs b/lib/src/lib.rs index e72f4bf9b..18cba4ac9 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -1,4 +1,6 @@ #![warn(unsafe_op_in_unsafe_fn)] + +mod traits; mod util; pub mod arrays; diff --git a/lib/src/replica.rs b/lib/src/replica.rs index 0d2a5af4b..7512d1f91 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -1,3 +1,4 @@ +use crate::traits::*; use crate::util::err_to_tcstring; use crate::{result::TCResult, status::TCStatus, string::TCString, task::TCTask, uuid::TCUuid}; use taskchampion::{Replica, StorageConfig, Uuid}; @@ -5,10 +6,25 @@ use taskchampion::{Replica, StorageConfig, Uuid}; /// A replica represents an instance of a user's task data, providing an easy interface /// for querying and modifying that data. /// -/// TCReplicas are not threadsafe. +/// # Error Handling /// /// When a `tc_replica_..` function that returns a TCResult returns TC_RESULT_ERROR, then /// `tc_replica_error` will return the error message. +/// +/// # Safety +/// +/// The `*TCReplica` returned from `tc_replica_new…` functions is owned by the caller and +/// must later be freed to avoid a memory leak. +/// +/// Any function taking a `*TCReplica` requires: +/// - the pointer must not be NUL; +/// - the pointer must be one previously returned from a tc_… function; +/// - the memory referenced by the pointer must never be modified by C code; and +/// - except for `tc_replica_free`, ownership of a `*TCReplica` remains with the caller. +/// +/// Once passed to `tc_replica_free`, a `*TCReplica` becmes invalid and must not be used again. +/// +/// TCReplicas are not threadsafe. pub struct TCReplica { /// The wrapped Replica inner: Replica, @@ -20,37 +36,9 @@ pub struct TCReplica { error: Option>, } +impl PassByPointer for TCReplica {} + impl TCReplica { - /// Borrow a TCReplica from C as an argument. - /// - /// # Safety - /// - /// The pointer must not be NULL. It is the caller's responsibility to ensure that the - /// lifetime assigned to the reference and the lifetime of the TCReplica itself do not outlive - /// the lifetime promised by C. - pub(crate) unsafe fn from_arg_ref<'a>(tcreplica: *mut TCReplica) -> &'a mut Self { - debug_assert!(!tcreplica.is_null()); - // SAFETY: see doc comment - unsafe { &mut *tcreplica } - } - - /// Take a TCReplica from C as an argument. - /// - /// # Safety - /// - /// The pointer must not be NULL and must point to a valid replica. The pointer becomes - /// invalid before this function returns and must not be used afterward. - pub(crate) unsafe fn from_arg(tcreplica: *mut TCReplica) -> Self { - debug_assert!(!tcreplica.is_null()); - // SAFETY: see doc comment - unsafe { *Box::from_raw(tcreplica) } - } - - /// Convert this to a return value for handing off to C. - pub(crate) fn return_val(self) -> *mut TCReplica { - Box::into_raw(Box::new(self)) - } - /// Mutably borrow the inner Replica pub(crate) fn borrow_mut(&mut self) -> &mut Replica { if self.mut_borrowed { @@ -85,10 +73,8 @@ fn wrap<'a, T, F>(rep: *mut TCReplica, f: F, err_value: T) -> T where F: FnOnce(&mut Replica) -> anyhow::Result, { - // SAFETY: - // - rep is not null (promised by caller) - // - rep outlives 'a (promised by caller) - let rep: &'a mut TCReplica = unsafe { TCReplica::from_arg_ref(rep) }; + // SAFETY: see type docstring + let rep: &'a mut TCReplica = unsafe { TCReplica::from_arg_ref_mut(rep) }; if rep.mut_borrowed { panic!("replica is borrowed and cannot be used"); } @@ -109,21 +95,19 @@ pub extern "C" fn tc_replica_new_in_memory() -> *mut TCReplica { let storage = StorageConfig::InMemory .into_storage() .expect("in-memory always succeeds"); - TCReplica::from(Replica::new(storage)).return_val() + // SAFETY: see type docstring + unsafe { TCReplica::from(Replica::new(storage)).return_val() } } -/// Create a new TCReplica with an on-disk database having the given filename. The filename must -/// not be NULL. On error, a string is written to the `error_out` parameter (if it is not NULL) and -/// NULL is returned. +/// Create a new TCReplica with an on-disk database having the given filename. On error, a string +/// is written to the `error_out` parameter (if it is not NULL) and NULL is returned. #[no_mangle] pub extern "C" fn tc_replica_new_on_disk<'a>( path: *mut TCString, error_out: *mut *mut TCString, ) -> *mut TCReplica { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let path = unsafe { TCString::from_arg(path) }; + // SAFETY: see TCString docstring + let path = unsafe { TCString::take_from_arg(path) }; let storage_res = StorageConfig::OnDisk { taskdb_dir: path.to_path_buf(), } @@ -141,7 +125,8 @@ pub extern "C" fn tc_replica_new_on_disk<'a>( } }; - TCReplica::from(Replica::new(storage)).return_val() + // SAFETY: see type docstring + unsafe { TCReplica::from(Replica::new(storage)).return_val() } } // TODO: tc_replica_all_tasks @@ -153,11 +138,11 @@ pub extern "C" fn tc_replica_new_on_disk<'a>( /// Returns NULL when the task does not exist, and on error. Consult tc_replica_error /// to distinguish the two conditions. #[no_mangle] -pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, uuid: TCUuid) -> *mut TCTask { +pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, tcuuid: TCUuid) -> *mut TCTask { wrap( rep, |rep| { - let uuid: Uuid = uuid.into(); + let uuid = Uuid::from_arg(tcuuid); if let Some(task) = rep.get_task(uuid)? { Ok(TCTask::from(task).return_val()) } else { @@ -170,8 +155,6 @@ pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, uuid: TCUuid) -> *mut /// Create a new task. The task must not already exist. /// -/// The description must not be NULL. -/// /// Returns the task, or NULL on error. #[no_mangle] pub extern "C" fn tc_replica_new_task( @@ -179,10 +162,8 @@ pub extern "C" fn tc_replica_new_task( status: TCStatus, description: *mut TCString, ) -> *mut TCTask { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let description = unsafe { TCString::from_arg(description) }; + // SAFETY: see TCString docstring + let description = unsafe { TCString::take_from_arg(description) }; wrap( rep, |rep| { @@ -199,12 +180,12 @@ pub extern "C" fn tc_replica_new_task( #[no_mangle] pub extern "C" fn tc_replica_import_task_with_uuid( rep: *mut TCReplica, - uuid: TCUuid, + tcuuid: TCUuid, ) -> *mut TCTask { wrap( rep, |rep| { - let uuid: Uuid = uuid.into(); + let uuid = Uuid::from_arg(tcuuid); let task = rep.import_task_with_uuid(uuid)?; Ok(TCTask::from(task).return_val()) }, @@ -241,12 +222,11 @@ pub extern "C" fn tc_replica_undo<'a>(rep: *mut TCReplica, undone_out: *mut i32) /// returned string. #[no_mangle] pub extern "C" fn tc_replica_error<'a>(rep: *mut TCReplica) -> *mut TCString<'static> { - // SAFETY: - // - rep is not null (promised by caller) - // - rep outlives 'a (promised by caller) - let rep: &'a mut TCReplica = unsafe { TCReplica::from_arg_ref(rep) }; + // SAFETY: see type docstring + let rep: &'a mut TCReplica = unsafe { TCReplica::from_arg_ref_mut(rep) }; if let Some(tcstring) = rep.error.take() { - tcstring.return_val() + // SAFETY: see TCString docstring + unsafe { tcstring.return_val() } } else { std::ptr::null_mut() } @@ -256,10 +236,8 @@ pub extern "C" fn tc_replica_error<'a>(rep: *mut TCReplica) -> *mut TCString<'st /// more than once. #[no_mangle] pub extern "C" fn tc_replica_free(rep: *mut TCReplica) { - // SAFETY: - // - rep is not NULL (promised by caller) - // - caller will not use the TCReplica after this (promised by caller) - let replica = unsafe { TCReplica::from_arg(rep) }; + // SAFETY: see type docstring + let replica = unsafe { TCReplica::take_from_arg(rep) }; if replica.mut_borrowed { panic!("replica is borrowed and cannot be freed"); } diff --git a/lib/src/string.rs b/lib/src/string.rs index e711966a3..d3f952189 100644 --- a/lib/src/string.rs +++ b/lib/src/string.rs @@ -1,24 +1,39 @@ +use crate::traits::*; use std::ffi::{CStr, CString, OsStr}; use std::os::unix::ffi::OsStrExt; use std::path::PathBuf; use std::str::Utf8Error; -/// TCString supports passing strings into and out of the TaskChampion API. A string can contain -/// embedded NUL characters. Strings containing such embedded NULs cannot be represented as a "C -/// string" and must be accessed using `tc_string_content_and_len` and `tc_string_clone_with_len`. -/// In general, these two functions should be used for handling arbitrary data, while more -/// convenient forms may be used where embedded NUL characters are impossible, such as in static -/// strings. +/// TCString supports passing strings into and out of the TaskChampion API. /// -/// Rust expects all strings to be UTF-8, and API functions will fail if given a TCString -/// containing invalid UTF-8. +/// # Rust Strings and C Strings /// -/// Unless specified otherwise, functions in this API take ownership of a TCString when it is given -/// as a function argument, and free the string before returning. Callers must not use or free -/// strings after passing them to such API functions. +/// A Rust string can contain embedded NUL characters, while C considers such a character to mark +/// the end of a string. Strings containing embedded NULs cannot be represented as a "C string" +/// and must be accessed using `tc_string_content_and_len` and `tc_string_clone_with_len`. In +/// general, these two functions should be used for handling arbitrary data, while more convenient +/// forms may be used where embedded NUL characters are impossible, such as in static strings. /// -/// When a TCString appears as a return value or output argument, it is the responsibility of the -/// caller to free the string. +/// # UTF-8 +/// +/// TaskChampion expects all strings to be valid UTF-8. `tc_string_…` functions will fail if given +/// a `*TCString` containing invalid UTF-8. +/// +/// # Safety +/// +/// When a `*TCString` appears as a return value or output argument, ownership is passed to the +/// caller. The caller must pass that ownerhsip back to another function or free the string. +/// +/// Any function taking a `*TCReplica` requires: +/// - the pointer must not be NUL; +/// - the pointer must be one previously returned from a tc_… function; and +/// - the memory referenced by the pointer must never be modified by C code. +/// +/// Unless specified otherwise, TaskChampion functions take ownership of a `*TCString` when it is +/// given as a function argument, and the pointer is invalid when the function returns. Callers +/// must not use or free TCStrings after passing them to such API functions. +/// +/// TCStrings are not threadsafe. pub enum TCString<'a> { CString(CString), CStr(&'a CStr), @@ -39,36 +54,9 @@ impl<'a> Default for TCString<'a> { } } +impl<'a> PassByPointer for TCString<'a> {} + impl<'a> TCString<'a> { - /// Take a TCString from C as an argument. - /// - /// C callers generally expect TC functions to take ownership of a string, which is what this - /// function does. - /// - /// # Safety - /// - /// The pointer must not be NULL. It is the caller's responsibility to ensure that the - /// lifetime assigned to the reference and the lifetime of the TCString itself do not outlive - /// the lifetime promised by C. - pub(crate) unsafe fn from_arg(tcstring: *mut TCString<'a>) -> Self { - debug_assert!(!tcstring.is_null()); - // SAFETY: see docstring - unsafe { *(Box::from_raw(tcstring)) } - } - - /// Borrow a TCString from C as an argument. - /// - /// # Safety - /// - /// The pointer must not be NULL. It is the caller's responsibility to ensure that the - /// lifetime assigned to the reference and the lifetime of the TCString itself do not outlive - /// the lifetime promised by C. - pub(crate) unsafe fn from_arg_ref(tcstring: *mut TCString<'a>) -> &'a mut Self { - debug_assert!(!tcstring.is_null()); - // SAFETY: see docstring - unsafe { &mut *tcstring } - } - /// Get a regular Rust &str for this value. pub(crate) fn as_str(&self) -> Result<&str, std::str::Utf8Error> { match self { @@ -95,11 +83,6 @@ impl<'a> TCString<'a> { let path: &OsStr = OsStr::from_bytes(self.as_bytes()); path.to_os_string().into() } - - /// Convert this to a return value for handing off to C. - pub(crate) fn return_val(self) -> *mut TCString<'a> { - Box::into_raw(Box::new(self)) - } } impl<'a> From for TCString<'a> { @@ -137,7 +120,8 @@ pub extern "C" fn tc_string_borrow(cstr: *const libc::c_char) -> *mut TCString<' // - cstr contains a valid NUL terminator (promised by caller) // - cstr's content will not change before it is destroyed (promised by caller) let cstr: &CStr = unsafe { CStr::from_ptr(cstr) }; - TCString::CStr(cstr).return_val() + // SAFETY: see docstring + unsafe { TCString::CStr(cstr).return_val() } } /// Create a new TCString by cloning the content of the given C string. The resulting TCString @@ -151,7 +135,8 @@ pub extern "C" fn tc_string_clone(cstr: *const libc::c_char) -> *mut TCString<'s // - cstr contains a valid NUL terminator (promised by caller) // - cstr's content will not change before it is destroyed (by C convention) let cstr: &CStr = unsafe { CStr::from_ptr(cstr) }; - TCString::CString(cstr.into()).return_val() + // SAFETY: see docstring + unsafe { TCString::CString(cstr.into()).return_val() } } /// Create a new TCString containing the given string with the given length. This allows creation @@ -176,14 +161,15 @@ pub extern "C" fn tc_string_clone_with_len( let vec = slice.to_vec(); // try converting to a string, which is the only variant that can contain embedded NULs. If // the bytes are not valid utf-8, store that information for reporting later. - match String::from_utf8(vec) { + let tcstring = match String::from_utf8(vec) { Ok(string) => TCString::String(string), Err(e) => { let (e, vec) = (e.utf8_error(), e.into_bytes()); TCString::InvalidUtf8(e, vec) } - } - .return_val() + }; + // SAFETY: see docstring + unsafe { tcstring.return_val() } } /// Get the content of the string as a regular C string. The given string must not be NULL. The @@ -200,11 +186,12 @@ pub extern "C" fn tc_string_content(tcstring: *mut TCString) -> *const libc::c_c // - tcstring is not NULL (promised by caller) // - lifetime of tcstring outlives the lifetime of this function // - lifetime of tcstring outlives the lifetime of the returned pointer (promised by caller) - let tcstring = unsafe { TCString::from_arg_ref(tcstring) }; + let tcstring = unsafe { TCString::from_arg_ref_mut(tcstring) }; // if we have a String, we need to consume it and turn it into // a CString. if matches!(tcstring, TCString::String(_)) { + // TODO: put this in a method if let TCString::String(string) = std::mem::take(tcstring) { match CString::new(string) { Ok(cstring) => { @@ -237,7 +224,8 @@ pub extern "C" fn tc_string_content(tcstring: *mut TCString) -> *const libc::c_c /// Get the content of the string as a pointer and length. The given string must not be NULL. /// This function can return any string, even one including NUL bytes or invalid UTF-8. The -/// returned buffer is valid until the TCString is freed or passed to another TC API function. +/// returned buffer is valid until the TCString is freed or passed to another TaskChampio +/// function. /// /// This function does _not_ take ownership of the TCString. #[no_mangle] @@ -274,5 +262,5 @@ pub extern "C" fn tc_string_free(tcstring: *mut TCString) { // SAFETY: // - tcstring is not NULL (promised by caller) // - caller is exclusive owner of tcstring (promised by caller) - drop(unsafe { TCString::from_arg(tcstring) }); + drop(unsafe { TCString::take_from_arg(tcstring) }); } diff --git a/lib/src/task.rs b/lib/src/task.rs index 6c305792d..87dcdd773 100644 --- a/lib/src/task.rs +++ b/lib/src/task.rs @@ -1,3 +1,4 @@ +use crate::traits::*; use crate::util::err_to_tcstring; use crate::{ arrays::TCTags, replica::TCReplica, result::TCResult, status::TCStatus, string::TCString, @@ -93,7 +94,8 @@ impl TCTask { // SAFETY: // - tcreplica is not null (promised by caller) // - tcreplica outlives the pointer in this variant (promised by caller) - let tcreplica_ref: &mut TCReplica = unsafe { TCReplica::from_arg_ref(tcreplica) }; + let tcreplica_ref: &mut TCReplica = + unsafe { TCReplica::from_arg_ref_mut(tcreplica) }; let rep_ref = tcreplica_ref.borrow_mut(); Inner::Mutable(task.into_mut(rep_ref), tcreplica) } @@ -112,7 +114,8 @@ impl TCTask { // - tcreplica is not null (promised by caller of to_mut, which created this // variant) // - tcreplica is still alive (promised by caller of to_mut) - let tcreplica_ref: &mut TCReplica = unsafe { TCReplica::from_arg_ref(tcreplica) }; + let tcreplica_ref: &mut TCReplica = + unsafe { TCReplica::from_arg_ref_mut(tcreplica) }; tcreplica_ref.release_borrow(); Inner::Immutable(task.into_immut()) } @@ -246,7 +249,7 @@ pub extern "C" fn tc_task_to_immut<'a>(task: *mut TCTask) { /// Get a task's UUID. #[no_mangle] pub extern "C" fn tc_task_get_uuid(task: *mut TCTask) -> TCUuid { - wrap(task, |task| task.get_uuid().into()) + wrap(task, |task| task.get_uuid().return_val()) } /// Get a task's status. @@ -263,7 +266,8 @@ pub extern "C" fn tc_task_get_status<'a>(task: *mut TCTask) -> TCStatus { pub extern "C" fn tc_task_get_description<'a>(task: *mut TCTask) -> *mut TCString<'static> { wrap(task, |task| { let descr: TCString = task.get_description().into(); - descr.return_val() + // SAFETY: see TCString docstring + unsafe { descr.return_val() } }) } @@ -298,13 +302,12 @@ pub extern "C" fn tc_task_is_active(task: *mut TCTask) -> bool { } /// Check if a task has the given tag. If the tag is invalid, this function will simply return -/// false with no error from `tc_task_error`. The given tag must not be NULL. +/// false, with no error from `tc_task_error`. +// TODO: why no error?? #[no_mangle] pub extern "C" fn tc_task_has_tag<'a>(task: *mut TCTask, tag: *mut TCString) -> bool { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let tcstring = unsafe { TCString::from_arg(tag) }; + // SAFETY: see TCString docstring + let tcstring = unsafe { TCString::take_from_arg(tag) }; wrap(task, |task| { if let Ok(tag) = Tag::try_from(tcstring) { task.has_tag(&tag) @@ -321,7 +324,8 @@ pub extern "C" fn tc_task_get_tags<'a>(task: *mut TCTask) -> TCTags { let tcstrings: Vec>> = task .get_tags() .map(|t| { - let t_ptr = TCString::from(t.as_ref()).return_val(); + // SAFETY: see TCString docstring + let t_ptr = unsafe { TCString::from(t.as_ref()).return_val() }; // SAFETY: t_ptr was just created and is not NULL unsafe { NonNull::new_unchecked(t_ptr) } }) @@ -355,10 +359,8 @@ pub extern "C" fn tc_task_set_description<'a>( task: *mut TCTask, description: *mut TCString, ) -> TCResult { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let description = unsafe { TCString::from_arg(description) }; + // SAFETY: see TCString docstring + let description = unsafe { TCString::take_from_arg(description) }; wrap_mut( task, |task| { @@ -466,10 +468,8 @@ pub extern "C" fn tc_task_delete(task: *mut TCTask) -> TCResult { /// Add a tag to a mutable task. #[no_mangle] pub extern "C" fn tc_task_add_tag(task: *mut TCTask, tag: *mut TCString) -> TCResult { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let tcstring = unsafe { TCString::from_arg(tag) }; + // SAFETY: see TCString docstring + let tcstring = unsafe { TCString::take_from_arg(tag) }; wrap_mut( task, |task| { @@ -484,10 +484,8 @@ pub extern "C" fn tc_task_add_tag(task: *mut TCTask, tag: *mut TCString) -> TCRe /// Remove a tag from a mutable task. #[no_mangle] pub extern "C" fn tc_task_remove_tag(task: *mut TCTask, tag: *mut TCString) -> TCResult { - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let tcstring = unsafe { TCString::from_arg(tag) }; + // SAFETY: see TCString docstring + let tcstring = unsafe { TCString::take_from_arg(tag) }; wrap_mut( task, |task| { @@ -516,7 +514,7 @@ pub extern "C" fn tc_task_error<'a>(task: *mut TCTask) -> *mut TCString<'static> // - task outlives 'a (promised by caller) let task: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; if let Some(tcstring) = task.error.take() { - tcstring.return_val() + unsafe { tcstring.return_val() } } else { std::ptr::null_mut() } diff --git a/lib/src/uuid.rs b/lib/src/uuid.rs index 9dcd3797b..8f57c7d1a 100644 --- a/lib/src/uuid.rs +++ b/lib/src/uuid.rs @@ -1,4 +1,5 @@ use crate::string::TCString; +use crate::traits::*; use libc; use taskchampion::Uuid; @@ -9,28 +10,30 @@ use taskchampion::Uuid; #[repr(C)] pub struct TCUuid([u8; 16]); -impl From for TCUuid { - fn from(uuid: Uuid) -> TCUuid { - TCUuid(*uuid.as_bytes()) - } -} +impl PassByValue for Uuid { + type CType = TCUuid; -impl From for Uuid { - fn from(uuid: TCUuid) -> Uuid { - Uuid::from_bytes(uuid.0) + unsafe fn from_ctype(arg: TCUuid) -> Self { + // SAFETY: + // - any 16-byte value is a valid Uuid + Uuid::from_bytes(arg.0) + } + + fn as_ctype(self) -> TCUuid { + TCUuid(*self.as_bytes()) } } /// Create a new, randomly-generated UUID. #[no_mangle] pub extern "C" fn tc_uuid_new_v4() -> TCUuid { - Uuid::new_v4().into() + Uuid::new_v4().return_val() } /// Create a new UUID with the nil value. #[no_mangle] pub extern "C" fn tc_uuid_nil() -> TCUuid { - Uuid::nil().into() + Uuid::nil().return_val() } // NOTE: this must be a simple constant so that cbindgen can evaluate it @@ -40,7 +43,7 @@ pub const TC_UUID_STRING_BYTES: usize = 36; /// Write the string representation of a TCUuid into the given buffer, which must be /// at least TC_UUID_STRING_BYTES long. No NUL terminator is added. #[no_mangle] -pub extern "C" fn tc_uuid_to_buf<'a>(uuid: TCUuid, buf: *mut libc::c_char) { +pub extern "C" fn tc_uuid_to_buf<'a>(tcuuid: TCUuid, buf: *mut libc::c_char) { debug_assert!(!buf.is_null()); // SAFETY: // - buf is valid for len bytes (by C convention) @@ -51,34 +54,34 @@ pub extern "C" fn tc_uuid_to_buf<'a>(uuid: TCUuid, buf: *mut libc::c_char) { let buf: &'a mut [u8] = unsafe { std::slice::from_raw_parts_mut(buf as *mut u8, ::uuid::adapter::Hyphenated::LENGTH) }; - let uuid: Uuid = uuid.into(); + let uuid: Uuid = Uuid::from_arg(tcuuid); uuid.to_hyphenated().encode_lower(buf); } /// Write the string representation of a TCUuid into the given buffer, which must be /// at least TC_UUID_STRING_BYTES long. No NUL terminator is added. #[no_mangle] -pub extern "C" fn tc_uuid_to_str(uuid: TCUuid) -> *mut TCString<'static> { - let uuid: Uuid = uuid.into(); +pub extern "C" fn tc_uuid_to_str(tcuuid: TCUuid) -> *mut TCString<'static> { + let uuid: Uuid = Uuid::from_arg(tcuuid); let s = uuid.to_string(); - TCString::from(s).return_val() + // SAFETY: see TCString docstring + unsafe { TCString::from(s).return_val() } } -/// Parse the given string as a UUID. The string must not be NULL. Returns false on failure. +/// Parse the given string as a UUID. Returns false on failure. #[no_mangle] pub extern "C" fn tc_uuid_from_str<'a>(s: *mut TCString, uuid_out: *mut TCUuid) -> bool { + // TODO: TCResult instead debug_assert!(!s.is_null()); debug_assert!(!uuid_out.is_null()); - // SAFETY: - // - tcstring is not NULL (promised by caller) - // - caller is exclusive owner of tcstring (implicitly promised by caller) - let s = unsafe { TCString::from_arg(s) }; + // SAFETY: see TCString docstring + let s = unsafe { TCString::take_from_arg(s) }; if let Ok(s) = s.as_str() { if let Ok(u) = Uuid::parse_str(s) { // SAFETY: // - uuid_out is not NULL (promised by caller) // - alignment is not required - unsafe { *uuid_out = u.into() }; + unsafe { u.to_arg_out(uuid_out) }; return true; } } diff --git a/lib/taskchampion.h b/lib/taskchampion.h index c001538a6..ccb8ff1db 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -41,30 +41,59 @@ typedef enum TCStatus { * A replica represents an instance of a user's task data, providing an easy interface * for querying and modifying that data. * - * TCReplicas are not threadsafe. + * # Error Handling * * When a `tc_replica_..` function that returns a TCResult returns TC_RESULT_ERROR, then * `tc_replica_error` will return the error message. + * + * # Safety + * + * The `*TCReplica` returned from `tc_replica_new…` functions is owned by the caller and + * must later be freed to avoid a memory leak. + * + * Any function taking a `*TCReplica` requires: + * - the pointer must not be NUL; + * - the pointer must be one previously returned from a tc_… function; + * - the memory referenced by the pointer must never be modified by C code; and + * - except for `tc_replica_free`, ownership of a `*TCReplica` remains with the caller. + * + * Once passed to `tc_replica_free`, a `*TCReplica` becmes invalid and must not be used again. + * + * TCReplicas are not threadsafe. */ typedef struct TCReplica TCReplica; /** - * TCString supports passing strings into and out of the TaskChampion API. A string can contain - * embedded NUL characters. Strings containing such embedded NULs cannot be represented as a "C - * string" and must be accessed using `tc_string_content_and_len` and `tc_string_clone_with_len`. - * In general, these two functions should be used for handling arbitrary data, while more - * convenient forms may be used where embedded NUL characters are impossible, such as in static - * strings. + * TCString supports passing strings into and out of the TaskChampion API. * - * Rust expects all strings to be UTF-8, and API functions will fail if given a TCString - * containing invalid UTF-8. + * # Rust Strings and C Strings * - * Unless specified otherwise, functions in this API take ownership of a TCString when it is given - * as a function argument, and free the string before returning. Callers must not use or free - * strings after passing them to such API functions. + * A Rust string can contain embedded NUL characters, while C considers such a character to mark + * the end of a string. Strings containing embedded NULs cannot be represented as a "C string" + * and must be accessed using `tc_string_content_and_len` and `tc_string_clone_with_len`. In + * general, these two functions should be used for handling arbitrary data, while more convenient + * forms may be used where embedded NUL characters are impossible, such as in static strings. * - * When a TCString appears as a return value or output argument, it is the responsibility of the - * caller to free the string. + * # UTF-8 + * + * TaskChampion expects all strings to be valid UTF-8. `tc_string_…` functions will fail if given + * a `*TCString` containing invalid UTF-8. + * + * # Safety + * + * When a `*TCString` appears as a return value or output argument, ownership is passed to the + * caller. The caller must pass that ownerhsip back to another function or free the string. + * + * Any function taking a `*TCReplica` requires: + * - the pointer must not be NUL; + * - the pointer must be one previously returned from a tc_… function; and + * - the memory referenced by the pointer must never be modified by C code. + * + * Unless specified otherwise, TaskChampion functions take ownership of a `*TCString` when it is + * given as a function argument, and the pointer is invalid when the function returns. Callers + * must not use or free TCStrings after passing them to such API functions. + * + * TCStrings are not threadsafe. */ typedef struct TCString TCString; @@ -138,9 +167,8 @@ void tc_tags_free(struct TCTags *tctags); struct TCReplica *tc_replica_new_in_memory(void); /** - * Create a new TCReplica with an on-disk database having the given filename. The filename must - * not be NULL. On error, a string is written to the `error_out` parameter (if it is not NULL) and - * NULL is returned. + * Create a new TCReplica with an on-disk database having the given filename. On error, a string + * is written to the `error_out` parameter (if it is not NULL) and NULL is returned. */ struct TCReplica *tc_replica_new_on_disk(struct TCString *path, struct TCString **error_out); @@ -150,13 +178,11 @@ struct TCReplica *tc_replica_new_on_disk(struct TCString *path, struct TCString * Returns NULL when the task does not exist, and on error. Consult tc_replica_error * to distinguish the two conditions. */ -struct TCTask *tc_replica_get_task(struct TCReplica *rep, struct TCUuid uuid); +struct TCTask *tc_replica_get_task(struct TCReplica *rep, struct TCUuid tcuuid); /** * Create a new task. The task must not already exist. * - * The description must not be NULL. - * * Returns the task, or NULL on error. */ struct TCTask *tc_replica_new_task(struct TCReplica *rep, @@ -168,7 +194,7 @@ struct TCTask *tc_replica_new_task(struct TCReplica *rep, * * Returns the task, or NULL on error. */ -struct TCTask *tc_replica_import_task_with_uuid(struct TCReplica *rep, struct TCUuid uuid); +struct TCTask *tc_replica_import_task_with_uuid(struct TCReplica *rep, struct TCUuid tcuuid); /** * Undo local operations until the most recent UndoPoint. @@ -239,7 +265,8 @@ const char *tc_string_content(struct TCString *tcstring); /** * Get the content of the string as a pointer and length. The given string must not be NULL. * This function can return any string, even one including NUL bytes or invalid UTF-8. The - * returned buffer is valid until the TCString is freed or passed to another TC API function. + * returned buffer is valid until the TCString is freed or passed to another TaskChampio + * function. * * This function does _not_ take ownership of the TCString. */ @@ -323,7 +350,7 @@ bool tc_task_is_active(struct TCTask *task); /** * Check if a task has the given tag. If the tag is invalid, this function will simply return - * false with no error from `tc_task_error`. The given tag must not be NULL. + * false, with no error from `tc_task_error`. */ bool tc_task_has_tag(struct TCTask *task, struct TCString *tag); @@ -417,16 +444,16 @@ struct TCUuid tc_uuid_nil(void); * Write the string representation of a TCUuid into the given buffer, which must be * at least TC_UUID_STRING_BYTES long. No NUL terminator is added. */ -void tc_uuid_to_buf(struct TCUuid uuid, char *buf); +void tc_uuid_to_buf(struct TCUuid tcuuid, char *buf); /** * Write the string representation of a TCUuid into the given buffer, which must be * at least TC_UUID_STRING_BYTES long. No NUL terminator is added. */ -struct TCString *tc_uuid_to_str(struct TCUuid uuid); +struct TCString *tc_uuid_to_str(struct TCUuid tcuuid); /** - * Parse the given string as a UUID. The string must not be NULL. Returns false on failure. + * Parse the given string as a UUID. Returns false on failure. */ bool tc_uuid_from_str(struct TCString *s, struct TCUuid *uuid_out);