From 0d68e65354fdb9d83a4d4e6270b68a6aeafb8209 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Tue, 25 Jan 2022 23:29:52 +0000 Subject: [PATCH] some polish on strings --- .../src/bindings_tests/replica.c | 4 +- integration-tests/src/bindings_tests/string.c | 4 +- integration-tests/src/bindings_tests/task.c | 2 +- lib/src/lib.rs | 1 + lib/src/replica.rs | 20 ++++++-- lib/src/result.rs | 10 ++++ lib/src/string.rs | 37 +++++++++++--- lib/taskchampion.h | 51 +++++++++++++++---- 8 files changed, 100 insertions(+), 29 deletions(-) create mode 100644 lib/src/result.rs diff --git a/integration-tests/src/bindings_tests/replica.c b/integration-tests/src/bindings_tests/replica.c index 71eef0efe..613b15666 100644 --- a/integration-tests/src/bindings_tests/replica.c +++ b/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); } diff --git a/integration-tests/src/bindings_tests/string.c b/integration-tests/src/bindings_tests/string.c index ad6c62559..9926d79bd 100644 --- a/integration-tests/src/bindings_tests/string.c +++ b/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)); diff --git a/integration-tests/src/bindings_tests/task.c b/integration-tests/src/bindings_tests/task.c index 9f77b8170..b0bbc4117 100644 --- a/integration-tests/src/bindings_tests/task.c +++ b/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)); diff --git a/lib/src/lib.rs b/lib/src/lib.rs index a97d0f732..649febbd9 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -1,4 +1,5 @@ pub mod replica; +pub mod result; pub mod status; pub mod string; pub mod task; diff --git a/lib/src/replica.rs b/lib/src/replica.rs index 08bab398f..7ae4cc286 100644 --- a/lib/src/replica.rs +++ b/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. diff --git a/lib/src/result.rs b/lib/src/result.rs new file mode 100644 index 000000000..e807e4690 --- /dev/null +++ b/lib/src/result.rs @@ -0,0 +1,10 @@ +/// A result combines a boolean success value with +/// an error response. It is equivalent to `Result`. +/// cbindgen:prefix-with-name +/// cbindgen:rename-all=ScreamingSnakeCase +#[repr(C)] +pub enum TCResult { + True, + False, + Error, +} diff --git a/lib/src/string.rs b/lib/src/string.rs index 1816d2a7d..1cb716b62 100644 --- a/lib/src/string.rs +++ b/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, diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 02e2284fd..9e3354c14 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -1,6 +1,16 @@ #include #include +/** + * A result combines a boolean success value with + * an error response. It is equivalent to `Result`. + */ +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);