From d24319179c64455e61f2973f1746e947548fc566 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 30 Jan 2022 23:53:12 +0000 Subject: [PATCH] TCFoo::from_arg to take from a pointer --- lib/src/replica.rs | 19 +++++++++++++++---- lib/src/task.rs | 22 +++++++++++++--------- lib/taskchampion.h | 3 ++- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/src/replica.rs b/lib/src/replica.rs index d9c28eb0b..a5643f045 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -29,7 +29,15 @@ impl TCReplica { &mut *tcreplica } - // TODO: from_arg_owned, use in drop + /// Take a TCReplica from C as an argument. + /// + /// # Safety + /// + /// The pointer must not be NULL. The pointer becomes invalid before this function returns. + pub(crate) unsafe fn from_arg(tcreplica: *mut TCReplica) -> Self { + debug_assert!(!tcreplica.is_null()); + *Box::from_raw(tcreplica) + } /// Convert this to a return value for handing off to C. pub(crate) fn return_val(self) -> *mut TCReplica { @@ -238,15 +246,18 @@ pub extern "C" fn tc_replica_error<'a>(rep: *mut TCReplica) -> *mut TCString<'st } } -/// Free a TCReplica. +/// Free a replica. The replica may not be used after this function returns and must not be freed +/// more than once. #[no_mangle] pub extern "C" fn tc_replica_free(rep: *mut TCReplica) { - let replica: &mut TCReplica = unsafe { TCReplica::from_arg_ref(rep) }; + // SAFETY: + // - rep is not NULL + // - caller will not use the TCReplica after this + let replica = unsafe { TCReplica::from_arg(rep) }; if replica.mut_borrowed { panic!("replica is borrowed and cannot be freed"); } drop(replica); - drop(unsafe { Box::from_raw(rep) }); } // TODO: tc_replica_rebuild_working_set diff --git a/lib/src/task.rs b/lib/src/task.rs index f929416d9..558843b17 100644 --- a/lib/src/task.rs +++ b/lib/src/task.rs @@ -49,7 +49,15 @@ impl TCTask { &mut *tctask } - // TODO: from_arg_owned, use in drop + /// Take a TCTask from C as an argument. + /// + /// # Safety + /// + /// The pointer must not be NULL. The pointer becomes invalid before this function returns. + pub(crate) unsafe fn from_arg<'a>(tctask: *mut TCTask) -> Self { + debug_assert!(!tctask.is_null()); + *Box::from_raw(tctask) + } /// Convert a TCTask to a return value for handing off to C. pub(crate) fn return_val(self) -> *mut TCTask { @@ -272,8 +280,10 @@ pub extern "C" fn tc_task_set_description<'a>( /// The restriction that the task must be immutable may be lifted (TODO) #[no_mangle] pub extern "C" fn tc_task_free<'a>(task: *mut TCTask) { - // convert the task to immutable before freeing - let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; + // SAFETY: + // - rep is not NULL + // - caller will not use the TCTask after this + let tctask = unsafe { TCTask::from_arg(task) }; if !matches!(tctask, TCTask::Immutable(_)) { // this limit is in place because we require the caller to supply a pointer // to the replica to make a task immutable, and no such pointer is available @@ -281,10 +291,4 @@ pub extern "C" fn tc_task_free<'a>(task: *mut TCTask) { panic!("Task must be immutable when freed"); } drop(tctask); - - // 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) }); } diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 18aa00faf..cda313171 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -138,7 +138,8 @@ enum TCResult tc_replica_undo(struct TCReplica *rep); struct TCString *tc_replica_error(struct TCReplica *rep); /** - * Free a TCReplica. + * Free a replica. The replica may not be used after this function returns and must not be freed + * more than once. */ void tc_replica_free(struct TCReplica *rep);