diff --git a/lib/src/replica.rs b/lib/src/replica.rs index 6bb89b209..1d8014424 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -50,14 +50,18 @@ pub extern "C" fn tc_replica_new_in_memory() -> *mut TCReplica { })) } -/// Create a new TCReplica with an on-disk database. 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. 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. #[no_mangle] pub extern "C" fn tc_replica_new_on_disk<'a>( path: *mut TCString, error_out: *mut *mut TCString, ) -> *mut TCReplica { - let path = TCString::from_arg(path); + // 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) }; let storage_res = StorageConfig::OnDisk { taskdb_dir: path.to_path_buf(), } @@ -107,6 +111,8 @@ 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( @@ -114,10 +120,13 @@ 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) }; wrap( rep, |rep| { - let description = TCString::from_arg(description); let task = rep.new_task(status.into(), description.as_str()?.to_string())?; Ok(TCTask::as_ptr(task)) }, diff --git a/lib/src/string.rs b/lib/src/string.rs index 1614a0c02..df0eebddb 100644 --- a/lib/src/string.rs +++ b/lib/src/string.rs @@ -14,7 +14,8 @@ use std::str::Utf8Error; /// containing invalid UTF-8. /// /// 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. +/// as a function argument, and free the string before returning. Callers must not use or free +/// strings after passing them to such API functions. /// /// When a TCString appears as a return value or output argument, it is the responsibility of the /// caller to free the string. @@ -39,17 +40,31 @@ impl<'a> Default 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. - pub(crate) fn from_arg(tcstring: *mut TCString<'a>) -> Self { + /// 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()); - *(unsafe { Box::from_raw(tcstring) }) + *(Box::from_raw(tcstring)) } /// Borrow a TCString from C as an argument. - pub(crate) fn from_arg_ref(tcstring: *mut TCString<'a>) -> &'a mut Self { + /// + /// # 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()); - unsafe { &mut *tcstring } + &mut *tcstring } /// Get a regular Rust &str for this value. @@ -97,11 +112,12 @@ impl<'a> From<&str> for TCString<'a> { } } -/// Create a new TCString referencing the given C string. The C string must remain valid until -/// after the TCString is freed. It's typically easiest to ensure this by using a static string. +/// Create a new TCString referencing the given C string. The C string must remain valid and +/// unchanged until after the TCString is freed. It's typically easiest to ensure this by using a +/// static string. /// -/// NOTE: this function does _not_ take responsibility for freeing the C string itself. The -/// underlying string can be freed once the TCString referencing it has been freed. +/// NOTE: this function does _not_ take responsibility for freeing the given C string. The +/// given string can be freed once the TCString referencing it has been freed. /// /// For example: /// @@ -112,6 +128,12 @@ impl<'a> From<&str> for TCString<'a> { /// ``` #[no_mangle] pub extern "C" fn tc_string_borrow(cstr: *const libc::c_char) -> *mut TCString<'static> { + debug_assert!(!cstr.is_null()); + // SAFETY: + // - cstr is not NULL (promised by caller, verified by assertion) + // - cstr's lifetime exceeds that of the TCString (promised by caller) + // - 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() } @@ -120,6 +142,12 @@ pub extern "C" fn tc_string_borrow(cstr: *const libc::c_char) -> *mut TCString<' /// is independent of the given string, which can be freed or overwritten immediately. #[no_mangle] pub extern "C" fn tc_string_clone(cstr: *const libc::c_char) -> *mut TCString<'static> { + debug_assert!(!cstr.is_null()); + // SAFETY: + // - cstr is not NULL (promised by caller, verified by assertion) + // - cstr's lifetime exceeds that of this function (by C convention) + // - 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() } @@ -127,11 +155,21 @@ pub extern "C" fn tc_string_clone(cstr: *const libc::c_char) -> *mut TCString<'s /// Create a new TCString containing the given string with the given length. This allows creation /// of strings containing embedded NUL characters. As with `tc_string_clone`, the resulting /// TCString is independent of the passed buffer, which may be reused or freed immediately. +/// +/// The given length must be less than half the maximum value of usize. #[no_mangle] pub extern "C" fn tc_string_clone_with_len( buf: *const libc::c_char, len: usize, ) -> *mut TCString<'static> { + debug_assert!(!buf.is_null()); + debug_assert!(len < isize::MAX as usize); + // SAFETY: + // - buf is valid for len bytes (by C convention) + // - (no alignment requirements for a byte slice) + // - content of buf will not be mutated during the lifetime of this slice (lifetime + // does not outlive this function call) + // - the length of the buffer is less than isize::MAX (promised by caller) let slice = unsafe { std::slice::from_raw_parts(buf as *const u8, len) }; let vec = slice.to_vec(); // try converting to a string, which is the only variant that can contain embedded NULs. If @@ -156,7 +194,11 @@ pub extern "C" fn tc_string_clone_with_len( /// This function does _not_ take ownership of the TCString. #[no_mangle] pub extern "C" fn tc_string_content(tcstring: *mut TCString) -> *const libc::c_char { - let tcstring = TCString::from_arg_ref(tcstring); + // SAFETY: + // - 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) }; // if we have a String, we need to consume it and turn it into // a CString. @@ -201,9 +243,13 @@ pub extern "C" fn tc_string_content_with_len( tcstring: *mut TCString, len_out: *mut usize, ) -> *const libc::c_char { - debug_assert!(!tcstring.is_null()); + // SAFETY: + // - 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) }; debug_assert!(!len_out.is_null()); - let tcstring = TCString::from_arg_ref(tcstring); + let bytes = match tcstring { TCString::CString(cstring) => cstring.as_bytes(), TCString::String(string) => string.as_bytes(), @@ -211,13 +257,20 @@ pub extern "C" fn tc_string_content_with_len( TCString::InvalidUtf8(_, ref v) => v.as_ref(), TCString::None => unreachable!(), }; + // SAFETY: + // - len_out is not NULL (checked by assertion, promised by caller) + // - len_out points to valid memory (promised by caller) + // - len_out is properly aligned (C convention) unsafe { *len_out = bytes.len() }; bytes.as_ptr() as *const libc::c_char } -/// Free a TCString. +/// Free a TCString. The given string must not be NULL. The string must not be used +/// after this function returns, and must not be freed more than once. #[no_mangle] -pub extern "C" fn tc_string_free(string: *mut TCString) { - debug_assert!(!string.is_null()); - drop(unsafe { Box::from_raw(string) }); +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) }); } diff --git a/lib/src/uuid.rs b/lib/src/uuid.rs index f25399909..535c03086 100644 --- a/lib/src/uuid.rs +++ b/lib/src/uuid.rs @@ -59,13 +59,15 @@ pub extern "C" fn tc_uuid_to_str(uuid: TCUuid) -> *mut TCString<'static> { TCString::from(s).return_val() } -/// Parse the given value as a UUID. The value must be exactly TC_UUID_STRING_BYTES long. Returns -/// false on failure. +/// Parse the given string as a UUID. The string must not be NULL. Returns false on failure. #[no_mangle] pub extern "C" fn tc_uuid_from_str<'a>(s: *mut TCString, uuid_out: *mut TCUuid) -> bool { debug_assert!(!s.is_null()); debug_assert!(!uuid_out.is_null()); - let s = TCString::from_arg(s); + // 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) }; if let Ok(s) = s.as_str() { if let Ok(u) = Uuid::parse_str(s) { unsafe { *uuid_out = u.into() }; diff --git a/lib/taskchampion.h b/lib/taskchampion.h index bf746289c..070b366e0 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -45,7 +45,8 @@ typedef struct TCReplica TCReplica; * containing invalid UTF-8. * * 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. + * as a function argument, and free the string before returning. Callers must not use or free + * strings after passing them to such API functions. * * When a TCString appears as a return value or output argument, it is the responsibility of the * caller to free the string. @@ -82,8 +83,9 @@ extern const size_t TC_UUID_STRING_BYTES; struct TCReplica *tc_replica_new_in_memory(void); /** - * Create a new TCReplica with an on-disk database. 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. 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. */ struct TCReplica *tc_replica_new_on_disk(struct TCString *path, struct TCString **error_out); @@ -98,6 +100,8 @@ struct TCTask *tc_replica_get_task(struct TCReplica *rep, struct TCUuid uuid); /** * 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, @@ -132,11 +136,12 @@ struct TCString *tc_replica_error(struct TCReplica *rep); void tc_replica_free(struct TCReplica *rep); /** - * Create a new TCString referencing the given C string. The C string must remain valid until - * after the TCString is freed. It's typically easiest to ensure this by using a static string. + * Create a new TCString referencing the given C string. The C string must remain valid and + * unchanged until after the TCString is freed. It's typically easiest to ensure this by using a + * static string. * - * NOTE: this function does _not_ take responsibility for freeing the C string itself. The - * underlying string can be freed once the TCString referencing it has been freed. + * NOTE: this function does _not_ take responsibility for freeing the given C string. The + * given string can be freed once the TCString referencing it has been freed. * * For example: * @@ -158,6 +163,8 @@ struct TCString *tc_string_clone(const char *cstr); * Create a new TCString containing the given string with the given length. This allows creation * of strings containing embedded NUL characters. As with `tc_string_clone`, the resulting * TCString is independent of the passed buffer, which may be reused or freed immediately. + * + * The given length must be less than half the maximum value of usize. */ struct TCString *tc_string_clone_with_len(const char *buf, size_t len); @@ -183,9 +190,10 @@ const char *tc_string_content(struct TCString *tcstring); const char *tc_string_content_with_len(struct TCString *tcstring, size_t *len_out); /** - * Free a TCString. + * Free a TCString. The given string must not be NULL. The string must not be used + * after this function returns, and must not be freed more than once. */ -void tc_string_free(struct TCString *string); +void tc_string_free(struct TCString *tcstring); /** * Get a task's UUID. @@ -231,8 +239,7 @@ void tc_uuid_to_buf(struct TCUuid uuid, char *buf); struct TCString *tc_uuid_to_str(struct TCUuid uuid); /** - * Parse the given value as a UUID. The value must be exactly TC_UUID_STRING_BYTES long. Returns - * false on failure. + * Parse the given string as a UUID. The string must not be NULL. Returns false on failure. */ bool tc_uuid_from_str(struct TCString *s, struct TCUuid *uuid_out);