From 914017b46c49e712431a1427c9754151845894c0 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 10 Feb 2022 00:10:39 +0000 Subject: [PATCH] tc_replica_all_tasks --- .../src/bindings_tests/replica.c | 44 ++++++++++++ lib/src/lib.rs | 1 + lib/src/replica.rs | 49 +++++++++++-- lib/src/task.rs | 16 +++-- lib/src/tasklist.rs | 72 +++++++++++++++++++ lib/taskchampion.h | 37 ++++++++++ 6 files changed, 207 insertions(+), 12 deletions(-) create mode 100644 lib/src/tasklist.rs diff --git a/integration-tests/src/bindings_tests/replica.c b/integration-tests/src/bindings_tests/replica.c index 11fc6d85c..41cf2dfe8 100644 --- a/integration-tests/src/bindings_tests/replica.c +++ b/integration-tests/src/bindings_tests/replica.c @@ -73,6 +73,49 @@ static void test_replica_task_creation(void) { tc_replica_free(rep); } +// a replica with tasks in it returns an appropriate list of tasks +static void test_replica_all_tasks(void) { + TCReplica *rep = tc_replica_new_in_memory(); + TEST_ASSERT_NULL(tc_replica_error(rep)); + + TCTask *task = tc_replica_new_task( + rep, + TC_STATUS_PENDING, + tc_string_borrow("task1")); + TEST_ASSERT_NOT_NULL(task); + tc_task_free(task); + + task = tc_replica_new_task( + rep, + TC_STATUS_PENDING, + tc_string_borrow("task2")); + TEST_ASSERT_NOT_NULL(task); + tc_task_free(task); + + TCTaskList tasks = tc_replica_all_tasks(rep); + TEST_ASSERT_NOT_NULL(tasks.items); + TEST_ASSERT_EQUAL(2, tasks.len); + + int seen1, seen2 = false; + for (size_t i = 0; i < tasks.len; i++) { + TCTask *task = tasks.items[i]; + TCString *descr = tc_task_get_description(task); + if (0 == strcmp(tc_string_content(descr), "task1")) { + seen1 = true; + } else if (0 == strcmp(tc_string_content(descr), "task2")) { + seen2 = true; + } + tc_string_free(descr); + } + TEST_ASSERT_TRUE(seen1); + TEST_ASSERT_TRUE(seen2); + + tc_task_list_free(&tasks); + TEST_ASSERT_NULL(tasks.items); + + tc_replica_free(rep); +} + // importing a task succeeds and the resulting task looks good static void test_replica_task_import(void) { TCReplica *rep = tc_replica_new_in_memory(); @@ -124,6 +167,7 @@ int replica_tests(void) { RUN_TEST(test_replica_undo_empty); RUN_TEST(test_replica_undo_empty_null_undone_out); RUN_TEST(test_replica_task_creation); + RUN_TEST(test_replica_all_tasks); RUN_TEST(test_replica_task_import); RUN_TEST(test_replica_get_task_not_found); return UNITY_END(); diff --git a/lib/src/lib.rs b/lib/src/lib.rs index f60142262..f9e353d21 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -12,4 +12,5 @@ pub mod status; pub mod string; pub mod stringlist; pub mod task; +pub mod tasklist; pub mod uuid; diff --git a/lib/src/replica.rs b/lib/src/replica.rs index 07cb3f7a8..b17c256a1 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -1,6 +1,10 @@ use crate::traits::*; use crate::util::err_to_tcstring; -use crate::{result::TCResult, status::TCStatus, string::TCString, task::TCTask, uuid::TCUuid}; +use crate::{ + result::TCResult, status::TCStatus, string::TCString, task::TCTask, tasklist::TCTaskList, + uuid::TCUuid, +}; +use std::ptr::NonNull; use taskchampion::{Replica, StorageConfig}; /// A replica represents an instance of a user's task data, providing an easy interface @@ -129,7 +133,34 @@ pub unsafe extern "C" fn tc_replica_new_on_disk<'a>( unsafe { TCReplica::from(Replica::new(storage)).return_val() } } -// TODO: tc_replica_all_tasks +/// Get a list of all tasks in the replica. +/// +/// Returns a TCTaskList with a NULL items field on error. +#[no_mangle] +pub unsafe extern "C" fn tc_replica_all_tasks(rep: *mut TCReplica) -> TCTaskList { + wrap( + rep, + |rep| { + // note that the Replica API returns a hashmap here, but we discard + // the keys and return a simple array. The task UUIDs are available + // from task.get_uuid(), so information is not lost. + let tasks: Vec<_> = rep + .all_tasks()? + .drain() + .map(|(_uuid, t)| { + NonNull::new( + // SAFETY: see TCTask docstring + unsafe { TCTask::from(t).return_val() }, + ) + .expect("TCTask::return_val returned NULL") + }) + .collect(); + Ok(TCTaskList::return_val(tasks)) + }, + TCTaskList::null_value(), + ) +} + // TODO: tc_replica_all_task_uuids // TODO: tc_replica_working_set @@ -145,7 +176,8 @@ pub unsafe extern "C" fn tc_replica_get_task(rep: *mut TCReplica, tcuuid: TCUuid // SAFETY: see TCUuid docstring let uuid = unsafe { TCUuid::from_arg(tcuuid) }; if let Some(task) = rep.get_task(uuid)? { - Ok(TCTask::from(task).return_val()) + // SAFETY: caller promises to free this task + Ok(unsafe { TCTask::from(task).return_val() }) } else { Ok(std::ptr::null_mut()) } @@ -169,7 +201,8 @@ pub unsafe extern "C" fn tc_replica_new_task( rep, |rep| { let task = rep.new_task(status.into(), description.as_str()?.to_string())?; - Ok(TCTask::from(task).return_val()) + // SAFETY: caller promises to free this task + Ok(unsafe { TCTask::from(task).return_val() }) }, std::ptr::null_mut(), ) @@ -189,7 +222,8 @@ pub unsafe extern "C" fn tc_replica_import_task_with_uuid( // SAFETY: see TCUuid docstring let uuid = unsafe { TCUuid::from_arg(tcuuid) }; let task = rep.import_task_with_uuid(uuid)?; - Ok(TCTask::from(task).return_val()) + // SAFETY: caller promises to free this task + Ok(unsafe { TCTask::from(task).return_val() }) }, std::ptr::null_mut(), ) @@ -202,7 +236,10 @@ pub unsafe extern "C" fn tc_replica_import_task_with_uuid( /// If undone_out is not NULL, then on success it is set to 1 if operations were undone, or 0 if /// there are no operations that can be done. #[no_mangle] -pub unsafe extern "C" fn tc_replica_undo<'a>(rep: *mut TCReplica, undone_out: *mut i32) -> TCResult { +pub unsafe extern "C" fn tc_replica_undo<'a>( + rep: *mut TCReplica, + undone_out: *mut i32, +) -> TCResult { wrap( rep, |rep| { diff --git a/lib/src/task.rs b/lib/src/task.rs index a69aedf04..c244081ed 100644 --- a/lib/src/task.rs +++ b/lib/src/task.rs @@ -49,7 +49,10 @@ enum Inner { Invalid, } +impl PassByPointer for TCTask {} + impl TCTask { + /* /// Borrow a TCTask from C as an argument. /// /// # Safety @@ -78,6 +81,7 @@ impl TCTask { pub(crate) fn return_val(self) -> *mut TCTask { Box::into_raw(Box::new(self)) } + */ /// Make an immutable TCTask into a mutable TCTask. Does nothing if the task /// is already mutable. @@ -140,7 +144,7 @@ where // SAFETY: // - task is not null (promised by caller) // - task outlives 'a (promised by caller) - let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; + let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; let task: &'a Task = match &tctask.inner { Inner::Immutable(t) => t, Inner::Mutable(t, _) => t.deref(), @@ -160,7 +164,7 @@ where // SAFETY: // - task is not null (promised by caller) // - task outlives 'a (promised by caller) - let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; + let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; let task: &'a mut TaskMut = match tctask.inner { Inner::Immutable(_) => panic!("Task is immutable"), Inner::Mutable(ref mut t, _) => t, @@ -222,7 +226,7 @@ pub unsafe extern "C" fn tc_task_to_mut<'a>(task: *mut TCTask, tcreplica: *mut T // SAFETY: // - task is not null (promised by caller) // - task outlives 'a (promised by caller) - let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; + let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; // SAFETY: // - tcreplica is not NULL (promised by caller) // - tcreplica lives until later call to to_immut via tc_task_to_immut (promised by caller, @@ -240,7 +244,7 @@ pub unsafe extern "C" fn tc_task_to_immut<'a>(task: *mut TCTask) { // SAFETY: // - task is not null (promised by caller) // - task outlives 'a (promised by caller) - let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; + let tctask: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; tctask.to_immut(); } @@ -516,7 +520,7 @@ pub unsafe extern "C" fn tc_task_error<'a>(task: *mut TCTask) -> *mut TCString<' // SAFETY: // - task is not null (promised by caller) // - task outlives 'a (promised by caller) - let task: &'a mut TCTask = unsafe { TCTask::from_arg_ref(task) }; + let task: &'a mut TCTask = unsafe { TCTask::from_arg_ref_mut(task) }; if let Some(tcstring) = task.error.take() { unsafe { tcstring.return_val() } } else { @@ -533,7 +537,7 @@ pub unsafe extern "C" fn tc_task_free<'a>(task: *mut TCTask) { // SAFETY: // - rep is not NULL (promised by caller) // - caller will not use the TCTask after this (promised by caller) - let mut tctask = unsafe { TCTask::from_arg(task) }; + let mut tctask = unsafe { TCTask::take_from_arg(task) }; // convert to immut if it was mutable tctask.to_immut(); diff --git a/lib/src/tasklist.rs b/lib/src/tasklist.rs new file mode 100644 index 000000000..eaba75ca1 --- /dev/null +++ b/lib/src/tasklist.rs @@ -0,0 +1,72 @@ +use crate::task::TCTask; +use crate::traits::*; +use std::ptr::NonNull; + +/// TCTaskList represents a list of tasks. +/// +/// The content of this struct must be treated as read-only. +#[repr(C)] +pub struct TCTaskList { + /// number of tasks in items + len: libc::size_t, + + /// total size of items (internal use only) + _capacity: libc::size_t, + + /// array of pointers representing each task. these remain owned by the TCTaskList instance and + /// will be freed by tc_task_list_free. This pointer is never NULL for a valid TCTaskList, + /// and the *TCTaskList at indexes 0..len-1 are not NULL. + items: *const NonNull, +} + +impl PointerArray for TCTaskList { + type Element = TCTask; + + unsafe fn from_raw_parts(items: *const NonNull, len: usize, cap: usize) -> Self { + TCTaskList { + len, + _capacity: cap, + items, + } + } + + fn into_raw_parts(self) -> (*const NonNull, usize, usize) { + (self.items, self.len, self._capacity) + } +} + +/// Free a TCTaskList instance. The instance, and all TCTaskList it contains, must not be used after +/// this call. +/// +/// When this call returns, the `items` pointer will be NULL, signalling an invalid TCTaskList. +#[no_mangle] +pub unsafe extern "C" fn tc_task_list_free(tctasks: *mut TCTaskList) { + debug_assert!(!tctasks.is_null()); + // SAFETY: + // - *tctasks is a valid TCTaskList (caller promises to treat it as read-only) + let tasks = unsafe { TCTaskList::take_from_arg(tctasks, TCTaskList::null_value()) }; + TCTaskList::drop_pointer_vector(tasks); +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn empty_array_has_non_null_pointer() { + let tctasks = TCTaskList::return_val(Vec::new()); + assert!(!tctasks.items.is_null()); + assert_eq!(tctasks.len, 0); + assert_eq!(tctasks._capacity, 0); + } + + #[test] + fn free_sets_null_pointer() { + let mut tctasks = TCTaskList::return_val(Vec::new()); + // SAFETY: testing expected behavior + unsafe { tc_task_list_free(&mut tctasks) }; + assert!(tctasks.items.is_null()); + assert_eq!(tctasks.len, 0); + assert_eq!(tctasks._capacity, 0); + } +} diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 4ac06acfd..e59a9c59d 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -117,6 +117,28 @@ typedef struct TCString TCString; */ typedef struct TCTask TCTask; +/** + * TCTaskList represents a list of tasks. + * + * The content of this struct must be treated as read-only. + */ +typedef struct TCTaskList { + /** + * number of tasks in items + */ + size_t len; + /** + * total size of items (internal use only) + */ + size_t _capacity; + /** + * array of pointers representing each task. these remain owned by the TCTaskList instance and + * will be freed by tc_task_list_free. This pointer is never NULL for a valid TCTaskList, + * and the *TCTaskList at indexes 0..len-1 are not NULL. + */ + struct TCTask *const *items; +} TCTaskList; + /** * TCUuid is used as a task identifier. Uuids do not contain any pointers and need not be freed. * Uuids are typically treated as opaque, but the bytes are available in big-endian format. @@ -164,6 +186,13 @@ struct TCReplica *tc_replica_new_in_memory(void); */ struct TCReplica *tc_replica_new_on_disk(struct TCString *path, struct TCString **error_out); +/** + * Get a list of all tasks in the replica. + * + * Returns a TCTaskList with a NULL items field on error. + */ +struct TCTaskList tc_replica_all_tasks(struct TCReplica *rep); + /** * Get an existing task by its UUID. * @@ -433,6 +462,14 @@ struct TCString *tc_task_error(struct TCTask *task); */ void tc_task_free(struct TCTask *task); +/** + * Free a TCTaskList instance. The instance, and all TCTaskList it contains, must not be used after + * this call. + * + * When this call returns, the `items` pointer will be NULL, signalling an invalid TCTaskList. + */ +void tc_task_list_free(struct TCTaskList *tctasks); + /** * Create a new, randomly-generated UUID. */