remove unnecessary string clone

binding-tests/string.cpp

@@ -10,20 +10,52 @@ TEST_CASE("creating borrowed strings does not crash") {
 TEST_CASE("creating cloned strings does not crash") {
     char *abcdef = strdup("abcdef");
     TCString *s = tc_string_clone(abcdef);
+    REQUIRE(s != NULL);
     free(abcdef);
+
     CHECK(strcmp(tc_string_content(s), "abcdef") == 0);
     tc_string_free(s);
 }
 
-TEST_CASE("strings echo back their content") {
+TEST_CASE("borrowed strings echo back their content") {
     TCString *s = tc_string_new("abcdef");
+    REQUIRE(s != NULL);
+
     CHECK(strcmp(tc_string_content(s), "abcdef") == 0);
+    size_t len;
+    const char *buf = tc_string_content_with_len(s, &len);
+    REQUIRE(buf != NULL);
+    CHECK(len == 6);
+    CHECK(strncmp(buf, "abcdef", len) == 0);
+    tc_string_free(s);
+}
+
+TEST_CASE("cloned strings echo back their content") {
+    char *orig = strdup("abcdef");
+    TCString *s = tc_string_clone(orig);
+    REQUIRE(s != NULL);
+    free(orig);
+
+    CHECK(strcmp(tc_string_content(s), "abcdef") == 0);
+
+    size_t len;
+    const char *buf = tc_string_content_with_len(s, &len);
+    REQUIRE(buf != NULL);
+    CHECK(len == 6);
+    CHECK(strncmp(buf, "abcdef", len) == 0);
     tc_string_free(s);
 }
 
 TEST_CASE("tc_string_content returns NULL for strings containing embedded NULs") {
     TCString *s = tc_string_clone_with_len("ab\0de", 5);
     REQUIRE(s != NULL);
+
     CHECK(tc_string_content(s) == NULL);
+
+    size_t len;
+    const char *buf = tc_string_content_with_len(s, &len);
+    REQUIRE(buf != NULL);
+    CHECK(len == 5);
+    CHECK(strncmp(buf, "ab\0de", len) == 0);
     tc_string_free(s);
 }

lib/build.rs

@@ -10,6 +10,7 @@ fn main() {
         .with_language(Language::C)
         .with_config(Config {
             cpp_compat: true,
+            usize_is_size_t: true,
             enumeration: EnumConfig {
                 // this appears to still default to true for C
                 enum_class: false,

lib/src/string.rs

@@ -6,11 +6,21 @@ use std::path::PathBuf;
 ///
 /// 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 otput argument.
+/// return value or output argument.
 pub enum TCString<'a> {
     CString(CString),
     CStr(&'a CStr),
     String(String),
+
+    /// None is the default value for TCString, but this variant is never seen by C code or by Rust
+    /// code outside of this module.
+    None,
+}
+
+impl<'a> Default for TCString<'a> {
+    fn default() -> Self {
+        TCString::None
+    }
 }
 
 impl<'a> TCString<'a> {
@@ -33,6 +43,7 @@ impl<'a> TCString<'a> {
             TCString::CString(cstring) => cstring.as_c_str().to_str(),
             TCString::CStr(cstr) => cstr.to_str(),
             TCString::String(string) => Ok(string.as_ref()),
+            TCString::None => unreachable!(),
         }
     }
 
@@ -41,6 +52,7 @@ impl<'a> TCString<'a> {
             TCString::CString(cstring) => cstring.as_bytes(),
             TCString::CStr(cstr) => cstr.to_bytes(),
             TCString::String(string) => string.as_bytes(),
+            TCString::None => unreachable!(),
         }
     }
 
@@ -101,33 +113,66 @@ pub extern "C" fn tc_string_clone_with_len(
 }
 
 /// Get the content of the string as a regular C string.  The given string must not be NULL.  The
-/// returned value may be NULL if the string contains NUL bytes.
+/// returned value is NULL if the string contains NUL bytes.  The returned string is valid until
+/// the TCString is freed or passed to another TC API function.
+///
 /// This function does _not_ take ownership of the TCString.
 #[no_mangle]
 pub extern "C" fn tc_string_content(tcstring: *mut TCString) -> *const libc::c_char {
     let tcstring = TCString::from_arg_ref(tcstring);
     // if we have a String, we need to consume it and turn it into
     // a CString.
-    if let TCString::String(string) = tcstring {
-        // TODO: get rid of this clone
-        match CString::new(string.clone()) {
+    if matches!(tcstring, TCString::String(_)) {
+        if let TCString::String(string) = std::mem::take(tcstring) {
+            match CString::new(string) {
                 Ok(cstring) => {
                     *tcstring = TCString::CString(cstring);
                 }
-            Err(_) => {
-                // TODO: could recover the underlying String
+                Err(nul_err) => {
+                    // recover the underlying String from the NulError
+                    let original_bytes = nul_err.into_vec();
+                    // SAFETY: original_bytes just came from a String, so must be valid utf8
+                    let string = unsafe { String::from_utf8_unchecked(original_bytes) };
+                    *tcstring = TCString::String(string);
+
+                    // and return NULL as advertized
                     return std::ptr::null();
                 }
             }
+        } else {
+            unreachable!()
+        }
     }
 
     match tcstring {
         TCString::CString(cstring) => cstring.as_ptr(),
-        TCString::String(_) => unreachable!(), // just converted this
+        TCString::String(_) => unreachable!(), // just converted to CString
         TCString::CStr(cstr) => cstr.as_ptr(),
+        TCString::None => unreachable!(),
     }
 }
 
+/// Get the content of the string as a pointer and length.  The given string must not be NULL.
+/// This function can return any string, even one including NUL bytes.  The returned string is
+/// valid until the TCString is freed or passed to another TC API function.
+///
+/// This function does _not_ take ownership of the TCString.
+#[no_mangle]
+pub extern "C" fn tc_string_content_with_len(
+    tcstring: *mut TCString,
+    len_out: *mut usize,
+) -> *const libc::c_char {
+    let tcstring = TCString::from_arg_ref(tcstring);
+    let bytes = match tcstring {
+        TCString::CString(cstring) => cstring.as_bytes(),
+        TCString::String(string) => string.as_bytes(),
+        TCString::CStr(cstr) => cstr.to_bytes(),
+        TCString::None => unreachable!(),
+    };
+    unsafe { *len_out = bytes.len() };
+    bytes.as_ptr() as *const libc::c_char
+}
+
 /// Free a TCString.
 #[no_mangle]
 pub extern "C" fn tc_string_free(string: *mut TCString) {

lib/taskchampion.h

@@ -1,4 +1,5 @@
 #include <cstdarg>
+#include <cstddef>
 #include <cstdint>
 #include <cstdlib>
 #include <ostream>
@@ -40,7 +41,7 @@ struct TCUuid {
 
 extern "C" {
 
-extern const uintptr_t TC_UUID_STRING_BYTES;
+extern const size_t TC_UUID_STRING_BYTES;
 
 /// Create a new TCReplica.
 ///
@@ -81,13 +82,22 @@ 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.
-TCString *tc_string_clone_with_len(const char *buf, uintptr_t len);
+TCString *tc_string_clone_with_len(const char *buf, size_t len);
 
 /// Get the content of the string as a regular C string.  The given string must not be NULL.  The
-/// returned value may be NULL if the string contains NUL bytes.
+/// returned value is NULL if the string contains NUL bytes.  The returned string is valid until
+/// the TCString is freed or passed to another TC API function.
+///
 /// This function does _not_ take ownership of the TCString.
 const char *tc_string_content(TCString *tcstring);
 
+/// Get the content of the string as a pointer and length.  The given string must not be NULL.
+/// This function can return any string, even one including NUL bytes.  The returned string is
+/// valid until the TCString is freed or passed to another TC API function.
+///
+/// This function does _not_ take ownership of the TCString.
+const char *tc_string_content_with_len(TCString *tcstring, size_t *len_out);
+
 /// Free a TCString.
 void tc_string_free(TCString *string);