From 3dd2ae501194c248e943347717ca4167da4d3a57 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Wed, 2 Feb 2022 03:28:54 +0000 Subject: [PATCH] implement TCTags as an array --- integration-tests/src/bindings_tests/task.c | 49 ++++++++++- lib/src/arrays.rs | 98 +++++++++++++++++++++ lib/src/lib.rs | 1 + lib/src/string.rs | 4 +- lib/src/task.rs | 42 ++++++++- lib/taskchampion.h | 40 +++++++++ 6 files changed, 226 insertions(+), 8 deletions(-) create mode 100644 lib/src/arrays.rs diff --git a/integration-tests/src/bindings_tests/task.c b/integration-tests/src/bindings_tests/task.c index 74dcfcda7..2ef89204c 100644 --- a/integration-tests/src/bindings_tests/task.c +++ b/integration-tests/src/bindings_tests/task.c @@ -247,8 +247,8 @@ static void test_task_done_and_delete(void) { tc_replica_free(rep); } -// adding tags to a task works, and invalid tags are rejected -static void task_task_add_tag(void) { +// adding and removing tags to a task works, and invalid tags are rejected +static void test_task_add_remove_has_tag(void) { TCReplica *rep = tc_replica_new_in_memory(); TEST_ASSERT_NULL(tc_replica_error(rep)); @@ -287,6 +287,48 @@ static void task_task_add_tag(void) { TEST_ASSERT_TRUE(tc_task_has_tag(task, tc_string_borrow("next"))); + // remove the tag + TEST_ASSERT_EQUAL(TC_RESULT_OK, tc_task_remove_tag(task, tc_string_borrow("next"))); + TEST_ASSERT_NULL(tc_task_error(task)); + + TEST_ASSERT_FALSE(tc_task_has_tag(task, tc_string_borrow("next"))); + + tc_task_free(task); + tc_replica_free(rep); +} + +// get_tags returns the list of tags +static void test_task_get_tags(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("my task")); + TEST_ASSERT_NOT_NULL(task); + + tc_task_to_mut(task, rep); + + TEST_ASSERT_EQUAL(TC_RESULT_OK, tc_task_add_tag(task, tc_string_borrow("next"))); + + TCTags tags = tc_task_get_tags(task); + + int found_pending = false, found_next = false; + for (size_t i = 0; i < tags.num_tags; i++) { + if (strcmp("PENDING", tc_string_content(tags.tags[i])) == 0) { + found_pending = true; + } + if (strcmp("next", tc_string_content(tags.tags[i])) == 0) { + found_next = true; + } + } + TEST_ASSERT_TRUE(found_pending); + TEST_ASSERT_TRUE(found_next); + + tc_tags_free(&tags); + TEST_ASSERT_NULL(tags.tags); + tc_task_free(task); tc_replica_free(rep); } @@ -303,6 +345,7 @@ int task_tests(void) { RUN_TEST(test_task_get_set_wait_and_is_waiting); RUN_TEST(test_task_start_stop_is_active); RUN_TEST(test_task_done_and_delete); - RUN_TEST(task_task_add_tag); + RUN_TEST(test_task_add_remove_has_tag); + RUN_TEST(test_task_get_tags); return UNITY_END(); } diff --git a/lib/src/arrays.rs b/lib/src/arrays.rs new file mode 100644 index 000000000..e94ead43b --- /dev/null +++ b/lib/src/arrays.rs @@ -0,0 +1,98 @@ +use crate::string::TCString; +use std::ptr::NonNull; + +// TODO: generalize to TCStrings? + +/// TCTags represents a list of tags associated with a task. +/// +/// The content of this struct must be treated as read-only. +/// +/// The lifetime of a TCTags instance is independent of the task, and it +/// will remain valid even if the task is freed. +#[repr(C)] +pub struct TCTags { + // TODO: can we use NonNull here? + /// strings representing each tag. these remain owned by the + /// TCTags instance and will be freed by tc_tags_free. + tags: *const NonNull>, + /// number of tags in tags + num_tags: libc::size_t, + /// total size of tags (internal use only) + _capacity: libc::size_t, +} + +impl Default for TCTags { + fn default() -> Self { + Self { + tags: std::ptr::null_mut(), + num_tags: 0, + _capacity: 0, + } + } +} + +impl TCTags { + /// Create a Vec of TCStrings into a TCTags instance. + pub(crate) fn new(tags: Vec>>) -> Self { + // emulate Vec::into_raw_parts(): + // - disable dropping the Vec with ManuallyDrop + // - extract ptr, len, and capacity using those methods + let mut tags = std::mem::ManuallyDrop::new(tags); + Self { + tags: tags.as_mut_ptr(), + num_tags: tags.len(), + _capacity: tags.capacity(), + } + } + + /// Convert a TCTags to a Vec<_>. + /// + /// # Safety + /// + /// Tags must be _exactly_ as created by [`new`] + unsafe fn into_vec(self) -> Vec>> { + // SAFETY: + // + // * tags.tags needs to have been previously allocated via Vec<*mut TCString> + // * TCString needs to have the same size and alignment as what ptr was allocated with. + // * length needs to be less than or equal to capacity. + // * capacity needs to be the capacity that the pointer was allocated with. + // * vec elements are not NULL + // + // All of this is true for a value returned from `new`, which the caller promised + // not to change. + unsafe { Vec::from_raw_parts(self.tags as *mut _, self.num_tags, self._capacity) } + } +} + +/// Free a TCTags instance. The given pointer must not be NULL. The instance must not be used +/// after this call. +#[no_mangle] +pub extern "C" fn tc_tags_free<'a>(tctags: *mut TCTags) { + debug_assert!(!tctags.is_null()); + // SAFETY: + // - tctags is not NULL + // - tctags is valid (caller promises it has not been changed) + // - caller will not use the TCTags after this (promised by caller) + let tctags: &'a mut TCTags = unsafe { &mut *tctags }; + + debug_assert!(!tctags.tags.is_null()); + + // replace the caller's TCTags with one containing a NULL pointer + let tctags: TCTags = std::mem::take(tctags); + + // convert to a regular Vec + // SAFETY: + // - tctags is exactly as returned from TCTags::new (promised by caller) + let mut vec: Vec<_> = unsafe { tctags.into_vec() }; + + // 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()) }); + } + + drop(vec); +} diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 7706f8d5c..e72f4bf9b 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -1,6 +1,7 @@ #![warn(unsafe_op_in_unsafe_fn)] mod util; +pub mod arrays; pub mod replica; pub mod result; pub mod status; diff --git a/lib/src/string.rs b/lib/src/string.rs index 163c2caf8..e711966a3 100644 --- a/lib/src/string.rs +++ b/lib/src/string.rs @@ -108,8 +108,8 @@ impl<'a> From for TCString<'a> { } } -impl<'a> From<&str> for TCString<'a> { - fn from(string: &str) -> TCString<'a> { +impl<'a> From<&str> for TCString<'static> { + fn from(string: &str) -> TCString<'static> { TCString::String(string.to_string()) } } diff --git a/lib/src/task.rs b/lib/src/task.rs index 4caa9a0c7..6c305792d 100644 --- a/lib/src/task.rs +++ b/lib/src/task.rs @@ -1,13 +1,17 @@ use crate::util::err_to_tcstring; use crate::{ - replica::TCReplica, result::TCResult, status::TCStatus, string::TCString, uuid::TCUuid, + arrays::TCTags, replica::TCReplica, result::TCResult, status::TCStatus, string::TCString, + uuid::TCUuid, }; use chrono::{DateTime, TimeZone, Utc}; use std::convert::TryFrom; use std::ops::Deref; +use std::ptr::NonNull; use std::str::FromStr; use taskchampion::{Tag, Task, TaskMut}; +// TODO: use NonNull more + /// A task, as publicly exposed by this library. /// /// A task begins in "immutable" mode. It must be converted to "mutable" mode @@ -310,7 +314,22 @@ pub extern "C" fn tc_task_has_tag<'a>(task: *mut TCTask, tag: *mut TCString) -> }) } -// TODO: tc_task_get_tags +/// Get the tags for the task. The task must not be NULL. +#[no_mangle] +pub extern "C" fn tc_task_get_tags<'a>(task: *mut TCTask) -> TCTags { + wrap(task, |task| { + let tcstrings: Vec>> = task + .get_tags() + .map(|t| { + let t_ptr = TCString::from(t.as_ref()).return_val(); + // SAFETY: t_ptr was just created and is not NULL + unsafe { NonNull::new_unchecked(t_ptr) } + }) + .collect(); + TCTags::new(tcstrings) + }) +} + // TODO: tc_task_get_annotations // TODO: tc_task_get_uda // TODO: tc_task_get_udas @@ -462,7 +481,24 @@ pub extern "C" fn tc_task_add_tag(task: *mut TCTask, tag: *mut TCString) -> TCRe ) } -// TODO: tc_task_remove_tag +/// 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) }; + wrap_mut( + task, + |task| { + let tag = Tag::try_from(tcstring)?; + task.remove_tag(&tag)?; + Ok(TCResult::Ok) + }, + TCResult::Error, + ) +} + // TODO: tc_task_add_annotation // TODO: tc_task_remove_annotation // TODO: tc_task_set_uda diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 4a3a33b96..854119bcc 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -88,6 +88,30 @@ typedef struct TCString TCString; */ typedef struct TCTask TCTask; +/** + * TCTags represents a list of tags associated with a task. + * + * The content of this struct must be treated as read-only. + * + * The lifetime of a TCTags instance is independent of the task, and it + * will remain valid even if the task is freed. + */ +typedef struct TCTags { + /** + * strings representing each tag. these remain owned by the + * TCTags instance and will be freed by tc_tags_free. + */ + struct TCString *const *tags; + /** + * number of tags in tags + */ + size_t num_tags; + /** + * total size of tags (internal use only; do not change) + */ + size_t _capacity; +} TCTags; + /** * 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. @@ -101,6 +125,12 @@ typedef struct TCUuid { extern "C" { #endif // __cplusplus +/** + * Free a TCTags instance. The given pointer must not be NULL. The instance must not be used + * after this call. + */ +void tc_tags_free(struct TCTags *tctags); + /** * Create a new TCReplica with an in-memory database. The contents of the database will be * lost when it is freed. @@ -297,6 +327,11 @@ bool tc_task_is_active(struct TCTask *task); */ bool tc_task_has_tag(struct TCTask *task, struct TCString *tag); +/** + * Get the tags for the task. The task must not be NULL. + */ +struct TCTags tc_task_get_tags(struct TCTask *task); + /** * Set a mutable task's status. */ @@ -348,6 +383,11 @@ TCResult tc_task_delete(struct TCTask *task); */ TCResult tc_task_add_tag(struct TCTask *task, struct TCString *tag); +/** + * Remove a tag from a mutable task. + */ +TCResult tc_task_remove_tag(struct TCTask *task, struct TCString *tag); + /** * Get the latest error for a task, or NULL if the last operation succeeded. Subsequent calls * to this function will return NULL. The task pointer must not be NULL. The caller must free the