TCUuidList, refactor traits

integration-tests/src/bindings_tests/replica.c

@@ -73,30 +73,33 @@ static void test_replica_task_creation(void) {
     tc_replica_free(rep);
 }
 
-// a replica with tasks in it returns an appropriate list of tasks
+// a replica with tasks in it returns an appropriate list of tasks and list of uuids
 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(
+    TCTask *task1 = tc_replica_new_task(
             rep,
             TC_STATUS_PENDING,
             tc_string_borrow("task1"));
-    TEST_ASSERT_NOT_NULL(task);
+    TEST_ASSERT_NOT_NULL(task1);
-    tc_task_free(task);
+    TCUuid uuid1 = tc_task_get_uuid(task1);
+    tc_task_free(task1);
 
-    task = tc_replica_new_task(
+    TCTask *task2 = tc_replica_new_task(
             rep,
             TC_STATUS_PENDING,
             tc_string_borrow("task2"));
-    TEST_ASSERT_NOT_NULL(task);
+    TEST_ASSERT_NOT_NULL(task2);
-    tc_task_free(task);
+    TCUuid uuid2 = tc_task_get_uuid(task2);
+    tc_task_free(task2);
 
+    {
         TCTaskList tasks = tc_replica_all_tasks(rep);
         TEST_ASSERT_NOT_NULL(tasks.items);
         TEST_ASSERT_EQUAL(2, tasks.len);
 
-    int seen1, seen2 = false;
+        bool seen1, seen2 = false;
         for (size_t i = 0; i < tasks.len; i++) {
             TCTask *task = tasks.items[i];
             TCString *descr = tc_task_get_description(task);
@@ -112,6 +115,28 @@ static void test_replica_all_tasks(void) {
 
         tc_task_list_free(&tasks);
         TEST_ASSERT_NULL(tasks.items);
+    }
+
+    {
+        TCUuidList uuids = tc_replica_all_task_uuids(rep);
+        TEST_ASSERT_NOT_NULL(uuids.items);
+        TEST_ASSERT_EQUAL(2, uuids.len);
+
+        bool seen1, seen2 = false;
+        for (size_t i = 0; i < uuids.len; i++) {
+            TCUuid uuid = uuids.items[i];
+            if (0 == memcmp(&uuid1, &uuid, sizeof(TCUuid))) {
+                seen1 = true;
+            } else if (0 == memcmp(&uuid2, &uuid, sizeof(TCUuid))) {
+                seen2 = true;
+            }
+        }
+        TEST_ASSERT_TRUE(seen1);
+        TEST_ASSERT_TRUE(seen2);
+
+        tc_uuid_list_free(&uuids);
+        TEST_ASSERT_NULL(uuids.items);
+    }
 
     tc_replica_free(rep);
 }

lib/src/lib.rs

@@ -14,3 +14,4 @@ pub mod stringlist;
 pub mod task;
 pub mod tasklist;
 pub mod uuid;
+pub mod uuidlist;

lib/src/replica.rs

@@ -2,7 +2,7 @@ use crate::traits::*;
 use crate::util::err_to_tcstring;
 use crate::{
     result::TCResult, status::TCStatus, string::TCString, task::TCTask, tasklist::TCTaskList,
-    uuid::TCUuid,
+    uuid::TCUuid, uuidlist::TCUuidList,
 };
 use std::ptr::NonNull;
 use taskchampion::{Replica, StorageConfig};
@@ -161,7 +161,25 @@ pub unsafe extern "C" fn tc_replica_all_tasks(rep: *mut TCReplica) -> TCTaskList
     )
 }
 
-// TODO: tc_replica_all_task_uuids
+/// Get a list of all uuids for tasks in the replica.
+///
+/// Returns a TCUuidList with a NULL items field on error.
+#[no_mangle]
+pub unsafe extern "C" fn tc_replica_all_task_uuids(rep: *mut TCReplica) -> TCUuidList {
+    wrap(
+        rep,
+        |rep| {
+            let uuids: Vec<_> = rep
+                .all_task_uuids()?
+                .drain(..)
+                .map(|uuid| TCUuid::return_val(uuid))
+                .collect();
+            Ok(TCUuidList::return_val(uuids))
+        },
+        TCUuidList::null_value(),
+    )
+}
+
 // TODO: tc_replica_working_set
 
 /// Get an existing task by its UUID.

lib/src/stringlist.rs

@@ -19,10 +19,10 @@ pub struct TCStringList {
     items: *const NonNull<TCString<'static>>,
 }
 
-impl PointerArray for TCStringList {
+impl ValueArray for TCStringList {
-    type Element = TCString<'static>;
+    type Element = NonNull<TCString<'static>>;
 
-    unsafe fn from_raw_parts(items: *const NonNull<Self::Element>, len: usize, cap: usize) -> Self {
+    unsafe fn from_raw_parts(items: *const Self::Element, len: usize, cap: usize) -> Self {
         TCStringList {
             len,
             _capacity: cap,
@@ -30,7 +30,7 @@ impl PointerArray for TCStringList {
         }
     }
 
-    fn into_raw_parts(self) -> (*const NonNull<Self::Element>, usize, usize) {
+    fn into_raw_parts(self) -> (*const Self::Element, usize, usize) {
         (self.items, self.len, self._capacity)
     }
 }
@@ -45,7 +45,7 @@ pub unsafe extern "C" fn tc_string_list_free(tcstrings: *mut TCStringList) {
     // SAFETY:
     //  - *tcstrings is a valid TCStringList (caller promises to treat it as read-only)
     let strings = unsafe { TCStringList::take_from_arg(tcstrings, TCStringList::null_value()) };
-    TCStringList::drop_pointer_vector(strings);
+    TCStringList::drop_vector(strings);
 }
 
 #[cfg(test)]

lib/src/tasklist.rs

@@ -19,10 +19,10 @@ pub struct TCTaskList {
     items: *const NonNull<TCTask>,
 }
 
-impl PointerArray for TCTaskList {
+impl ValueArray for TCTaskList {
-    type Element = TCTask;
+    type Element = NonNull<TCTask>;
 
-    unsafe fn from_raw_parts(items: *const NonNull<Self::Element>, len: usize, cap: usize) -> Self {
+    unsafe fn from_raw_parts(items: *const Self::Element, len: usize, cap: usize) -> Self {
         TCTaskList {
             len,
             _capacity: cap,
@@ -30,7 +30,7 @@ impl PointerArray for TCTaskList {
         }
     }
 
-    fn into_raw_parts(self) -> (*const NonNull<Self::Element>, usize, usize) {
+    fn into_raw_parts(self) -> (*const Self::Element, usize, usize) {
         (self.items, self.len, self._capacity)
     }
 }
@@ -45,7 +45,7 @@ pub unsafe extern "C" fn tc_task_list_free(tctasks: *mut TCTaskList) {
     // 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);
+    TCTaskList::drop_vector(tasks);
 }
 
 #[cfg(test)]

lib/src/traits.rs

@@ -146,38 +146,100 @@ pub(crate) trait PassByPointer: Sized {
     }
 }
 
-/// Support for arrays of objects referenced by pointer.
+/// *mut P can be passed by value
+impl<P> PassByValue for *mut P
+where
+    P: PassByPointer + 'static,
+{
+    type RustType = &'static mut P;
+
+    unsafe fn from_ctype(self) -> Self::RustType {
+        // SAFETY:
+        //  - self must be a valid *mut P (promised by caller)
+        //  - TODO for 'static
+        unsafe { &mut *self }
+    }
+
+    /// Convert a Rust value to a C value.
+    fn as_ctype(arg: Self::RustType) -> Self {
+        arg
+    }
+}
+
+/// *const P can be passed by value
+impl<P> PassByValue for *const P
+where
+    P: PassByPointer + 'static,
+{
+    type RustType = &'static P;
+
+    unsafe fn from_ctype(self) -> Self::RustType {
+        // SAFETY:
+        //  - self must be a valid *mut P (promised by caller)
+        //  - TODO for 'static
+        unsafe { &*self }
+    }
+
+    /// Convert a Rust value to a C value.
+    fn as_ctype(arg: Self::RustType) -> Self {
+        arg
+    }
+}
+
+/// NonNull<P> can be passed by value
+impl<P> PassByValue for NonNull<P>
+where
+    P: PassByPointer + 'static,
+{
+    type RustType = &'static mut P;
+
+    unsafe fn from_ctype(mut self) -> Self::RustType {
+        // SAFETY:
+        //  - self must be a valid *mut P (promised by caller)
+        //  - TODO for 'static
+        unsafe { self.as_mut() }
+    }
+
+    /// Convert a Rust value to a C value.
+    fn as_ctype(arg: Self::RustType) -> Self {
+        NonNull::new(arg).expect("value must not be NULL")
+    }
+}
+
+/// Support for arrays of objects referenced by value.
 ///
 /// The underlying C type should have three fields, containing items, length, and capacity.  The
 /// required trait functions just fetch and set these fields.  The PassByValue trait will be
-/// implemented automatically, converting between the C type and `Vec<NonNull<Element>>`.  For most
+/// implemented automatically, converting between the C type and `Vec<Element>`.  For most cases,
-/// cases, it is only necessary to implement `tc_.._free` that first calls
+/// it is only necessary to implement `tc_.._free` that first calls
-/// `PassByValue::take_from_arg(arg, PointerArray::null_value())` to take the existing value and
+/// `PassByValue::take_from_arg(arg, ValueArray::null_value())` to take the existing value and
-/// replace it with the null value; then `PointerArray::drop_pointer_vector(..)` to drop the
+/// replace it with the null value; then `ValueArray::drop_value_vector(..)` to drop the resulting
-/// resulting vector and all of the objects it points to.
+/// vector and all of the objects it points to.
+///
+/// This can be used for objects referenced by pointer, too, with an Element type of `*const T`
 ///
 /// # Safety
 ///
 /// The C type must be documented as read-only.  None of the fields may be modified, nor anything
 /// in the `items` array.
 ///
-/// This class guarantees that the items pointer is non-NULL for any valid array (even when len=0),
+/// This class guarantees that the items pointer is non-NULL for any valid array (even when len=0).
-/// and that all pointers at indexes 0..len-1 are non-NULL.
+// TODO: rename Array
-pub(crate) trait PointerArray: Sized {
+pub(crate) trait ValueArray: Sized {
-    type Element: 'static + PassByPointer;
+    type Element: PassByValue;
 
-    /// Create a new PointerArray from the given items, len, and capacity.
+    /// Create a new ValueArray from the given items, len, and capacity.
     ///
     /// # Safety
     ///
     /// The arguments must either:
     ///  - be NULL, 0, and 0, respectively; or
     ///  - be valid for Vec::from_raw_parts
-    unsafe fn from_raw_parts(items: *const NonNull<Self::Element>, len: usize, cap: usize) -> Self;
+    unsafe fn from_raw_parts(items: *const Self::Element, len: usize, cap: usize) -> Self;
 
     /// Get the items, len, and capacity (in that order) for this instance.  These must be
     /// precisely the same values passed tearlier to `from_raw_parts`.
-    fn into_raw_parts(self) -> (*const NonNull<Self::Element>, usize, usize);
+    fn into_raw_parts(self) -> (*const Self::Element, usize, usize);
 
     /// Generate a NULL value.  By default this is a NULL items pointer with zero length and
     /// capacity.
@@ -187,36 +249,32 @@ pub(crate) trait PointerArray: Sized {
         unsafe { Self::from_raw_parts(std::ptr::null(), 0, 0) }
     }
 
-    /// Drop a vector of element pointers.  This is a convenience function for implementing
+    /// Drop a vector of elements.  This is a convenience function for implementing
     /// tc_.._free functions.
-    fn drop_pointer_vector(mut vec: Vec<NonNull<Self::Element>>) {
+    fn drop_vector(mut vec: Vec<Self::Element>) {
         // first, drop each of the elements in turn
-        for p in vec.drain(..) {
+        for e in vec.drain(..) {
-            // SAFETY:
+            // SAFETY: e is a valid Element (caller promisd not to change it)
-            //  - p is not NULL (NonNull)
+            drop(unsafe { PassByValue::from_arg(e) });
-            //  - p was generated by Rust (true for all arrays)
-            //  - p was not modified (all arrays are immutable from C)
-            //  - caller will not use this pointer again (promised by caller; p has been drain'd from
-            //    the vector)
-            drop(unsafe { PassByPointer::take_from_arg(p.as_ptr()) });
         }
+        // then drop the vector
         drop(vec);
     }
 }
 
 impl<A> PassByValue for A
 where
-    A: PointerArray,
+    A: ValueArray,
 {
-    type RustType = Vec<NonNull<A::Element>>;
+    type RustType = Vec<A::Element>;
 
     unsafe fn from_ctype(self) -> Self::RustType {
         let (items, len, cap) = self.into_raw_parts();
         debug_assert!(!items.is_null());
         // SAFETY:
-        //  - PointerArray::from_raw_parts requires that items, len, and cap be valid for
+        //  - ValueArray::from_raw_parts requires that items, len, and cap be valid for
         //    Vec::from_raw_parts if not NULL, and they are not NULL (as promised by caller)
-        //  - PointerArray::into_raw_parts returns precisely the values passed to from_raw_parts.
+        //  - ValueArray::into_raw_parts returns precisely the values passed to from_raw_parts.
         //  - those parts are passed to Vec::from_raw_parts here.
         unsafe { Vec::from_raw_parts(items as *mut _, len, cap) }
     }

lib/src/uuidlist.rs

@@ -0,0 +1,70 @@
+use crate::traits::*;
+use crate::uuid::TCUuid;
+
+/// TCUuidList represents a list of uuids.
+///
+/// The content of this struct must be treated as read-only.
+#[repr(C)]
+pub struct TCUuidList {
+    /// number of uuids in items
+    len: libc::size_t,
+
+    /// total size of items (internal use only)
+    _capacity: libc::size_t,
+
+    /// array of uuids. these remain owned by the TCUuidList instance and will be freed by
+    /// tc_uuid_list_free.  This pointer is never NULL for a valid TCUuidList.
+    items: *const TCUuid,
+}
+
+impl ValueArray for TCUuidList {
+    type Element = TCUuid;
+
+    unsafe fn from_raw_parts(items: *const Self::Element, len: usize, cap: usize) -> Self {
+        TCUuidList {
+            len,
+            _capacity: cap,
+            items,
+        }
+    }
+
+    fn into_raw_parts(self) -> (*const Self::Element, usize, usize) {
+        (self.items, self.len, self._capacity)
+    }
+}
+
+/// Free a TCUuidList instance.  The instance, and all TCUuids it contains, must not be used after
+/// this call.
+///
+/// When this call returns, the `items` pointer will be NULL, signalling an invalid TCUuidList.
+#[no_mangle]
+pub unsafe extern "C" fn tc_uuid_list_free(tcuuids: *mut TCUuidList) {
+    debug_assert!(!tcuuids.is_null());
+    // SAFETY:
+    //  - *tcuuids is a valid TCUuidList (caller promises to treat it as read-only)
+    let uuids = unsafe { TCUuidList::take_from_arg(tcuuids, TCUuidList::null_value()) };
+    TCUuidList::drop_vector(uuids);
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn empty_array_has_non_null_pointer() {
+        let tcuuids = TCUuidList::return_val(Vec::new());
+        assert!(!tcuuids.items.is_null());
+        assert_eq!(tcuuids.len, 0);
+        assert_eq!(tcuuids._capacity, 0);
+    }
+
+    #[test]
+    fn free_sets_null_pointer() {
+        let mut tcuuids = TCUuidList::return_val(Vec::new());
+        // SAFETY: testing expected behavior
+        unsafe { tc_uuid_list_free(&mut tcuuids) };
+        assert!(tcuuids.items.is_null());
+        assert_eq!(tcuuids.len, 0);
+        assert_eq!(tcuuids._capacity, 0);
+    }
+}

lib/taskchampion.h

@@ -148,6 +148,27 @@ typedef struct TCUuid {
   uint8_t bytes[16];
 } TCUuid;
 
+/**
+ * TCUuidList represents a list of uuids.
+ *
+ * The content of this struct must be treated as read-only.
+ */
+typedef struct TCUuidList {
+  /**
+   * number of uuids in items
+   */
+  size_t len;
+  /**
+   * total size of items (internal use only)
+   */
+  size_t _capacity;
+  /**
+   * array of uuids. these remain owned by the TCUuidList instance and will be freed by
+   * tc_uuid_list_free.  This pointer is never NULL for a valid TCUuidList.
+   */
+  const struct TCUuid *items;
+} TCUuidList;
+
 /**
  * TCStringList represents a list of strings.
  *
@@ -193,6 +214,13 @@ struct TCReplica *tc_replica_new_on_disk(struct TCString *path, struct TCString
  */
 struct TCTaskList tc_replica_all_tasks(struct TCReplica *rep);
 
+/**
+ * Get a list of all uuids for tasks in the replica.
+ *
+ * Returns a TCUuidList with a NULL items field on error.
+ */
+struct TCUuidList tc_replica_all_task_uuids(struct TCReplica *rep);
+
 /**
  * Get an existing task by its UUID.
  *
@@ -498,6 +526,14 @@ struct TCString *tc_uuid_to_str(struct TCUuid tcuuid);
  */
 TCResult tc_uuid_from_str(struct TCString *s, struct TCUuid *uuid_out);
 
+/**
+ * Free a TCUuidList instance.  The instance, and all TCUuids it contains, must not be used after
+ * this call.
+ *
+ * When this call returns, the `items` pointer will be NULL, signalling an invalid TCUuidList.
+ */
+void tc_uuid_list_free(struct TCUuidList *tcuuids);
+
 #ifdef __cplusplus
 } // extern "C"
 #endif // __cplusplus