more unsafe notations

lib/src/replica.rs

@@ -11,10 +11,18 @@ pub struct TCReplica {
     error: Option<TCString<'static>>,
 }
 
-/// Utility function to safely convert *mut TCReplica into &mut TCReplica
+impl TCReplica {
-fn rep_ref(rep: *mut TCReplica) -> &'static mut TCReplica {
+    /// Borrow a TCReplica from C as an argument.
-    debug_assert!(!rep.is_null());
+    ///
-    unsafe { &mut *rep }
+    /// # 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>(tcstring: *mut TCReplica) -> &'a mut Self {
+        debug_assert!(!tcstring.is_null());
+        &mut *tcstring
+    }
 }
 
 fn err_to_tcstring(e: impl std::string::ToString) -> TCString<'static> {
@@ -26,7 +34,10 @@ fn wrap<'a, T, F>(rep: *mut TCReplica, f: F, err_value: T) -> T
 where
     F: FnOnce(&mut Replica) -> anyhow::Result<T>,
 {
-    let rep: &'a mut TCReplica = rep_ref(rep);
+    // 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) };
     rep.error = None;
     match f(&mut rep.inner) {
         Ok(v) => v,
@@ -100,7 +111,7 @@ pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, uuid: TCUuid) -> *mut
         |rep| {
             let uuid: Uuid = uuid.into();
             if let Some(task) = rep.get_task(uuid)? {
-                Ok(TCTask::as_ptr(task))
+                Ok(TCTask::return_val(task))
             } else {
                 Ok(std::ptr::null_mut())
             }
@@ -128,7 +139,7 @@ pub extern "C" fn tc_replica_new_task(
         rep,
         |rep| {
             let task = rep.new_task(status.into(), description.as_str()?.to_string())?;
-            Ok(TCTask::as_ptr(task))
+            Ok(TCTask::return_val(task))
         },
         std::ptr::null_mut(),
     )
@@ -147,7 +158,7 @@ pub extern "C" fn tc_replica_import_task_with_uuid(
         |rep| {
             let uuid: Uuid = uuid.into();
             let task = rep.import_task_with_uuid(uuid)?;
-            Ok(TCTask::as_ptr(task))
+            Ok(TCTask::return_val(task))
         },
         std::ptr::null_mut(),
     )
@@ -174,12 +185,15 @@ pub extern "C" fn tc_replica_undo<'a>(rep: *mut TCReplica) -> TCResult {
     )
 }
 
-/// Get the latest error for a replica, or NULL if the last operation succeeded.
+/// Get the latest error for a replica, or NULL if the last operation succeeded.  Subsequent calls
-/// Subsequent calls to this function will return NULL.  The caller must free the
+/// to this function will return NULL.  The rep pointer must not be NULL.  The caller must free the
 /// returned string.
 #[no_mangle]
 pub extern "C" fn tc_replica_error<'a>(rep: *mut TCReplica) -> *mut TCString<'static> {
-    let rep: &'a mut TCReplica = rep_ref(rep);
+    // 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) };
     if let Some(tcstring) = rep.error.take() {
         tcstring.return_val()
     } else {

lib/src/task.rs

@@ -10,21 +10,31 @@ pub struct TCTask {
 }
 
 impl TCTask {
-    pub(crate) fn as_ptr(task: Task) -> *mut TCTask {
+    /// Borrow a TCTask 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 TCTask itself do not outlive
+    /// the lifetime promised by C.
+    pub(crate) unsafe fn from_arg_ref<'a>(tcstring: *const TCTask) -> &'a Self {
+        debug_assert!(!tcstring.is_null());
+        &*tcstring
+    }
+
+    /// Convert this to a return value for handing off to C.
+    pub(crate) fn return_val(task: Task) -> *mut TCTask {
         Box::into_raw(Box::new(TCTask { inner: task }))
     }
 }
 
-/// Utility function to safely convert *const TCTask into &Task
-fn task_ref(task: *const TCTask) -> &'static Task {
-    debug_assert!(!task.is_null());
-    unsafe { &(*task).inner }
-}
-
 /// Get a task's UUID.
 #[no_mangle]
 pub extern "C" fn tc_task_get_uuid<'a>(task: *const TCTask) -> TCUuid {
-    let task: &'a Task = task_ref(task);
+    // SAFETY:
+    //  - task is not NULL (promised by caller)
+    //  - task's lifetime exceeds this function (promised by caller)
+    let task: &'a Task = &unsafe { TCTask::from_arg_ref(task) }.inner;
     let uuid = task.get_uuid();
     uuid.into()
 }
@@ -32,7 +42,10 @@ pub extern "C" fn tc_task_get_uuid<'a>(task: *const TCTask) -> TCUuid {
 /// Get a task's status.
 #[no_mangle]
 pub extern "C" fn tc_task_get_status<'a>(task: *const TCTask) -> TCStatus {
-    let task: &'a Task = task_ref(task);
+    // SAFETY:
+    //  - task is not NULL (promised by caller)
+    //  - task's lifetime exceeds this function (promised by caller)
+    let task: &'a Task = &unsafe { TCTask::from_arg_ref(task) }.inner;
     task.get_status().into()
 }
 
@@ -45,7 +58,10 @@ pub extern "C" fn tc_task_get_status<'a>(task: *const TCTask) -> TCStatus {
 /// contains embedded NUL characters).
 #[no_mangle]
 pub extern "C" fn tc_task_get_description<'a>(task: *const TCTask) -> *mut TCString<'static> {
-    let task = task_ref(task);
+    // SAFETY:
+    //  - task is not NULL (promised by caller)
+    //  - task's lifetime exceeds this function (promised by caller)
+    let task: &'a Task = &unsafe { TCTask::from_arg_ref(task) }.inner;
     let descr: TCString = task.get_description().into();
     descr.return_val()
 }
@@ -64,9 +80,14 @@ pub extern "C" fn tc_task_get_description<'a>(task: *const TCTask) -> *mut TCStr
  * get_modified
  */
 
-/// Free a task.
+/// Free a task.  The given task must not be NULL.  The task must not be used after this function
+/// returns, and must not be freed more than once.
 #[no_mangle]
 pub extern "C" fn tc_task_free<'a>(task: *mut TCTask) {
     debug_assert!(!task.is_null());
+    // SAFETY:
+    //  - task is not NULL (promised by caller)
+    //  - task's lifetime exceeds the drop (promised by caller)
+    //  - task does not outlive this function (promised by caller)
     drop(unsafe { Box::from_raw(task) });
 }

lib/src/uuid.rs

@@ -43,6 +43,12 @@ pub static TC_UUID_STRING_BYTES: usize = ::uuid::adapter::Hyphenated::LENGTH;
 #[no_mangle]
 pub extern "C" fn tc_uuid_to_buf<'a>(uuid: TCUuid, buf: *mut libc::c_char) {
     debug_assert!(!buf.is_null());
+    // 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 buf: &'a mut [u8] = unsafe {
         std::slice::from_raw_parts_mut(buf as *mut u8, ::uuid::adapter::Hyphenated::LENGTH)
     };
@@ -70,6 +76,9 @@ pub extern "C" fn tc_uuid_from_str<'a>(s: *mut TCString, uuid_out: *mut TCUuid)
     let s = unsafe { TCString::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() };
             return true;
         }

lib/taskchampion.h

@@ -124,8 +124,8 @@ struct TCTask *tc_replica_import_task_with_uuid(struct TCReplica *rep, struct TC
 enum TCResult tc_replica_undo(struct TCReplica *rep);
 
 /**
- * Get the latest error for a replica, or NULL if the last operation succeeded.
+ * Get the latest error for a replica, or NULL if the last operation succeeded.  Subsequent calls
- * Subsequent calls to this function will return NULL.  The caller must free the
+ * to this function will return NULL.  The rep pointer must not be NULL.  The caller must free the
  * returned string.
  */
 struct TCString *tc_replica_error(struct TCReplica *rep);
@@ -212,7 +212,8 @@ enum TCStatus tc_task_get_status(const struct TCTask *task);
 struct TCString *tc_task_get_description(const struct TCTask *task);
 
 /**
- * Free a task.
+ * Free a task.  The given task must not be NULL.  The task must not be used after this function
+ * returns, and must not be freed more than once.
  */
 void tc_task_free(struct TCTask *task);