some polish on strings

integration-tests/src/bindings_tests/replica.c

@@ -13,7 +13,7 @@ static void test_replica_creation(void) {
 
 // creating an on-disk replica does not crash
 static void test_replica_creation_disk(void) {
-    TCReplica *rep = tc_replica_new_on_disk(tc_string_new("test-db"), NULL);
+    TCReplica *rep = tc_replica_new_on_disk(tc_string_borrow("test-db"), NULL);
     TEST_ASSERT_NOT_NULL(rep);
     TEST_ASSERT_NULL(tc_replica_error(rep));
     tc_replica_free(rep);
@@ -24,7 +24,7 @@ static void test_replica_undo_empty(void) {
     TCReplica *rep = tc_replica_new_in_memory();
     TEST_ASSERT_NULL(tc_replica_error(rep));
     int rv = tc_replica_undo(rep);
-    TEST_ASSERT_EQUAL(0, rv);
+    TEST_ASSERT_EQUAL(TC_RESULT_FALSE, rv);
     TEST_ASSERT_NULL(tc_replica_error(rep));
     tc_replica_free(rep);
 }

integration-tests/src/bindings_tests/string.c

@@ -5,7 +5,7 @@
 
 // creating strings does not crash
 static void test_string_creation(void) {
-    TCString *s = tc_string_new("abcdef");
+    TCString *s = tc_string_borrow("abcdef");
     tc_string_free(s);
 }
 
@@ -22,7 +22,7 @@ static void test_string_cloning(void) {
 
 // borrowed strings echo back their content
 static void test_string_borrowed_strings_echo(void) {
-    TCString *s = tc_string_new("abcdef");
+    TCString *s = tc_string_borrow("abcdef");
     TEST_ASSERT_NOT_NULL(s);
 
     TEST_ASSERT_EQUAL_STRING("abcdef", tc_string_content(s));

integration-tests/src/bindings_tests/task.c

@@ -11,7 +11,7 @@ static void test_task_creation(void) {
     TCTask *task = tc_replica_new_task(
             rep,
             TC_STATUS_PENDING,
-            tc_string_new("my task"));
+            tc_string_borrow("my task"));
     TEST_ASSERT_NOT_NULL(task);
 
     TEST_ASSERT_EQUAL(TC_STATUS_PENDING, tc_task_get_status(task));

lib/src/lib.rs

@@ -1,4 +1,5 @@
 pub mod replica;
+pub mod result;
 pub mod status;
 pub mod string;
 pub mod task;

lib/src/replica.rs

@@ -1,4 +1,4 @@
-use crate::{status::TCStatus, string::TCString, task::TCTask};
+use crate::{result::TCResult, status::TCStatus, string::TCString, task::TCTask};
 use taskchampion::{Replica, StorageConfig};
 
 /// A replica represents an instance of a user's task data, providing an easy interface
@@ -114,11 +114,21 @@ pub extern "C" fn tc_replica_new_task<'a>(
 
 /// Undo local operations until the most recent UndoPoint.
 ///
-/// Returns -1 on error, 0 if there are no local operations to undo, and 1 if operations were
-/// undone.
+/// Returns TC_RESULT_TRUE if an undo occurred, TC_RESULT_FALSE if there are no operations
+/// to be undone, or TC_RESULT_ERROR on error.
 #[no_mangle]
-pub extern "C" fn tc_replica_undo<'a>(rep: *mut TCReplica) -> i32 {
-    wrap(rep, |rep| Ok(if rep.undo()? { 1 } else { 0 }), -1)
+pub extern "C" fn tc_replica_undo<'a>(rep: *mut TCReplica) -> TCResult {
+    wrap(
+        rep,
+        |rep| {
+            Ok(if rep.undo()? {
+                TCResult::True
+            } else {
+                TCResult::False
+            })
+        },
+        TCResult::Error,
+    )
 }
 
 /// Get the latest error for a replica, or NULL if the last operation succeeded.

lib/src/result.rs

@@ -0,0 +1,10 @@
+/// A result combines a boolean success value with
+/// an error response.  It is equivalent to `Result<bool, ()>`.
+/// cbindgen:prefix-with-name
+/// cbindgen:rename-all=ScreamingSnakeCase
+#[repr(C)]
+pub enum TCResult {
+    True,
+    False,
+    Error,
+}

lib/src/string.rs

@@ -2,11 +2,20 @@ use std::ffi::{CStr, CString, OsStr};
 use std::os::unix::ffi::OsStrExt;
 use std::path::PathBuf;
 
-/// TCString supports passing strings into and out of the TaskChampion API.
+// TODO: is utf-8-ness always checked? (no) when?
+
+/// TCString supports passing strings into and out of the TaskChampion API.  A string must contain
+/// valid UTF-8, and can contain embedded NUL characters.  Strings containing such embedded NULs
+/// cannot be represented as a "C string" and must be accessed using `tc_string_content_and_len`
+/// and `tc_string_clone_with_len`.  In general, these two functions should be used for handling
+/// arbitrary data, while more convenient forms may be used where embedded NUL characters are
+/// impossible, such as in static strings.
 ///
-/// Unless specified otherwise, functions in this API take ownership of a TCString when it appears
-/// as a function argument, and transfer ownership to the caller when the TCString appears as a
-/// return value or output argument.
+/// Unless specified otherwise, functions in this API take ownership of a TCString when it is given
+/// as a function argument, and free the string before returning.  Thus the following is valid:
+///
+/// When a TCString appears as a return value or output argument, it is the responsibility of the
+/// caller to free the string.
 pub enum TCString<'a> {
     CString(CString),
     CStr(&'a CStr),
@@ -82,13 +91,24 @@ impl<'a> From<&str> for TCString<'a> {
 
 /// Create a new TCString referencing the given C string.  The C string must remain valid until
 /// after the TCString is freed.  It's typically easiest to ensure this by using a static string.
+///
+/// NOTE: this function does _not_ take responsibility for freeing the C string itself.
+/// The underlying string once the TCString has been freed.  Among other times, TCStrings are
+/// freed when they are passed to API functions (unless documented otherwise).  For example:
+///
+/// ```
+/// char *url = get_item_url(..); // dynamically allocate C string
+/// tc_task_annotate(task, tc_string_borrow(url)); // TCString created, passed, and freed
+/// free(url); // string is no longer referenced and can be freed
+/// ```
 #[no_mangle]
-pub extern "C" fn tc_string_new(cstr: *const libc::c_char) -> *mut TCString<'static> {
+pub extern "C" fn tc_string_borrow(cstr: *const libc::c_char) -> *mut TCString<'static> {
     let cstr: &CStr = unsafe { CStr::from_ptr(cstr) };
     TCString::CStr(cstr).return_val()
 }
 
-/// Create a new TCString by cloning the content of the given C string.
+/// Create a new TCString by cloning the content of the given C string.  The resulting TCString
+/// is independent of the given string, which can be freed or overwritten immediately.
 #[no_mangle]
 pub extern "C" fn tc_string_clone(cstr: *const libc::c_char) -> *mut TCString<'static> {
     let cstr: &CStr = unsafe { CStr::from_ptr(cstr) };
@@ -96,8 +116,9 @@ pub extern "C" fn tc_string_clone(cstr: *const libc::c_char) -> *mut TCString<'s
 }
 
 /// Create a new TCString containing the given string with the given length. This allows creation
-/// of strings containing embedded NUL characters.  If the given string is not valid UTF-8, this
-/// function will return NULL.
+/// of strings containing embedded NUL characters.  As with `tc_string_clone`, the resulting
+/// TCString is independent of the passed buffer, which may be reused or freed immediately.  If the
+/// given string is not valid UTF-8, this function will return NULL.
 #[no_mangle]
 pub extern "C" fn tc_string_clone_with_len(
     buf: *const libc::c_char,

lib/taskchampion.h

@@ -1,6 +1,16 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+/**
+ * A result combines a boolean success value with
+ * an error response.  It is equivalent to `Result<bool, ()>`.
+ */
+typedef enum TCResult {
+  TC_RESULT_TRUE,
+  TC_RESULT_FALSE,
+  TC_RESULT_ERROR,
+} TCResult;
+
 /**
  * The status of a task, as defined by the task data model.
  */
@@ -24,11 +34,18 @@ typedef enum TCStatus {
 typedef struct TCReplica TCReplica;
 
 /**
- * TCString supports passing strings into and out of the TaskChampion API.
+ * TCString supports passing strings into and out of the TaskChampion API.  A string must contain
+ * valid UTF-8, and can contain embedded NUL characters.  Strings containing such embedded NULs
+ * cannot be represented as a "C string" and must be accessed using `tc_string_content_and_len`
+ * and `tc_string_clone_with_len`.  In general, these two functions should be used for handling
+ * arbitrary data, while more convenient forms may be used where embedded NUL characters are
+ * impossible, such as in static strings.
  *
- * Unless specified otherwise, functions in this API take ownership of a TCString when it appears
- * as a function argument, and transfer ownership to the caller when the TCString appears as a
- * return value or output argument.
+ * Unless specified otherwise, functions in this API take ownership of a TCString when it is given
+ * as a function argument, and free the string before returning.  Thus the following is valid:
+ *
+ * When a TCString appears as a return value or output argument, it is the responsibility of the
+ * caller to free the string.
  */
 typedef struct TCString TCString;
 
@@ -79,10 +96,10 @@ struct TCTask *tc_replica_new_task(struct TCReplica *rep,
 /**
  * Undo local operations until the most recent UndoPoint.
  *
- * Returns -1 on error, 0 if there are no local operations to undo, and 1 if operations were
- * undone.
+ * Returns TC_RESULT_TRUE if an undo occurred, TC_RESULT_FALSE if there are no operations
+ * to be undone, or TC_RESULT_ERROR on error.
  */
-int32_t tc_replica_undo(struct TCReplica *rep);
+enum TCResult tc_replica_undo(struct TCReplica *rep);
 
 /**
  * Get the latest error for a replica, or NULL if the last operation succeeded.
@@ -99,18 +116,30 @@ void tc_replica_free(struct TCReplica *rep);
 /**
  * Create a new TCString referencing the given C string.  The C string must remain valid until
  * after the TCString is freed.  It's typically easiest to ensure this by using a static string.
+ *
+ * NOTE: this function does _not_ take responsibility for freeing the C string itself.
+ * The underlying string once the TCString has been freed.  Among other times, TCStrings are
+ * freed when they are passed to API functions (unless documented otherwise).  For example:
+ *
+ * ```
+ * char *url = get_item_url(..); // dynamically allocate C string
+ * tc_task_annotate(task, tc_string_borrow(url)); // TCString created, passed, and freed
+ * free(url); // string is no longer referenced and can be freed
+ * ```
  */
-struct TCString *tc_string_new(const char *cstr);
+struct TCString *tc_string_borrow(const char *cstr);
 
 /**
- * Create a new TCString by cloning the content of the given C string.
+ * Create a new TCString by cloning the content of the given C string.  The resulting TCString
+ * is independent of the given string, which can be freed or overwritten immediately.
  */
 struct TCString *tc_string_clone(const char *cstr);
 
 /**
  * Create a new TCString containing the given string with the given length. This allows creation
- * of strings containing embedded NUL characters.  If the given string is not valid UTF-8, this
- * function will return NULL.
+ * of strings containing embedded NUL characters.  As with `tc_string_clone`, the resulting
+ * TCString is independent of the passed buffer, which may be reused or freed immediately.  If the
+ * given string is not valid UTF-8, this function will return NULL.
  */
 struct TCString *tc_string_clone_with_len(const char *buf, size_t len);