diff --git a/binding-tests/string.cpp b/binding-tests/string.cpp index dd726e862..8b743b266 100644 --- a/binding-tests/string.cpp +++ b/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); } diff --git a/lib/build.rs b/lib/build.rs index 13eda1bc5..d1ebc7bf1 100644 --- a/lib/build.rs +++ b/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, diff --git a/lib/src/string.rs b/lib/src/string.rs index 3f9086d0e..1816d2a7d 100644 --- a/lib/src/string.rs +++ b/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()) { - Ok(cstring) => { - *tcstring = TCString::CString(cstring); - } - Err(_) => { - // TODO: could recover the underlying String - return std::ptr::null(); + 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(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) { diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 445916baf..ad42de148 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -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);