From e11506ee6a70fc2f14d5c3a45739c88c9400cc8e Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sun, 6 Feb 2022 23:05:33 +0000 Subject: [PATCH] always implement traits for C type --- lib/src/atomic.rs | 12 ++++++------ lib/src/replica.rs | 6 +++--- lib/src/string.rs | 2 +- lib/src/strings.rs | 28 ++++++++++++++-------------- lib/src/task.rs | 4 ++-- lib/src/traits.rs | 25 +++++++++++++------------ lib/src/uuid.rs | 22 +++++++++++----------- lib/taskchampion.h | 20 ++++++++++---------- 8 files changed, 60 insertions(+), 59 deletions(-) diff --git a/lib/src/atomic.rs b/lib/src/atomic.rs index 341f7b339..db8315922 100644 --- a/lib/src/atomic.rs +++ b/lib/src/atomic.rs @@ -3,13 +3,13 @@ use crate::traits::*; impl PassByValue for usize { - type CType = usize; + type RustType = usize; - unsafe fn from_ctype(arg: usize) -> usize { - arg - } - - fn as_ctype(self) -> usize { + unsafe fn from_ctype(self) -> usize { self } + + fn as_ctype(arg: usize) -> usize { + arg + } } diff --git a/lib/src/replica.rs b/lib/src/replica.rs index 131a8eb91..d393d95c2 100644 --- a/lib/src/replica.rs +++ b/lib/src/replica.rs @@ -1,7 +1,7 @@ use crate::traits::*; use crate::util::err_to_tcstring; use crate::{result::TCResult, status::TCStatus, string::TCString, task::TCTask, uuid::TCUuid}; -use taskchampion::{Replica, StorageConfig, Uuid}; +use taskchampion::{Replica, StorageConfig}; /// A replica represents an instance of a user's task data, providing an easy interface /// for querying and modifying that data. @@ -143,7 +143,7 @@ pub extern "C" fn tc_replica_get_task(rep: *mut TCReplica, tcuuid: TCUuid) -> *m rep, |rep| { // SAFETY: see TCUuid docstring - let uuid = unsafe { Uuid::from_arg(tcuuid) }; + let uuid = unsafe { TCUuid::from_arg(tcuuid) }; if let Some(task) = rep.get_task(uuid)? { Ok(TCTask::from(task).return_val()) } else { @@ -187,7 +187,7 @@ pub extern "C" fn tc_replica_import_task_with_uuid( rep, |rep| { // SAFETY: see TCUuid docstring - let uuid = unsafe { Uuid::from_arg(tcuuid) }; + let uuid = unsafe { TCUuid::from_arg(tcuuid) }; let task = rep.import_task_with_uuid(uuid)?; Ok(TCTask::from(task).return_val()) }, diff --git a/lib/src/string.rs b/lib/src/string.rs index 3e95e8f6e..138ff31ea 100644 --- a/lib/src/string.rs +++ b/lib/src/string.rs @@ -255,7 +255,7 @@ pub extern "C" fn tc_string_content_with_len( // - len_out is not NULL (promised by caller) // - len_out points to valid memory (promised by caller) // - len_out is properly aligned (C convention) - unsafe { bytes.len().to_arg_out(len_out) }; + unsafe { usize::to_arg_out(bytes.len(), len_out) }; bytes.as_ptr() as *const libc::c_char } diff --git a/lib/src/strings.rs b/lib/src/strings.rs index c9f30460d..7d7e5bda3 100644 --- a/lib/src/strings.rs +++ b/lib/src/strings.rs @@ -3,37 +3,37 @@ use crate::traits::*; use crate::util::{drop_pointer_array, vec_into_raw_parts}; use std::ptr::NonNull; -/// TCStrings represents a list of string. +/// TCStrings represents a list of strings. /// /// The content of this struct must be treated as read-only. #[repr(C)] pub struct TCStrings { - /// number of tags in items + /// number of strings in items len: libc::size_t, /// total size of items (internal use only) _capacity: libc::size_t, - /// strings representing each tag. these remain owned by the TCStrings instance and will be freed + /// strings representing each string. these remain owned by the TCStrings instance and will be freed /// by tc_strings_free. items: *const NonNull>, } -impl PassByValue for Vec>> { - type CType = TCStrings; +impl PassByValue for TCStrings { + type RustType = Vec>>; - unsafe fn from_ctype(arg: TCStrings) -> Self { + unsafe fn from_ctype(self) -> Self::RustType { // SAFETY: // - C treats TCStrings as read-only, so items, len, and _capacity all came // from a Vec originally. - unsafe { Vec::from_raw_parts(arg.items as *mut _, arg.len, arg._capacity) } + unsafe { Vec::from_raw_parts(self.items as *mut _, self.len, self._capacity) } } - fn as_ctype(self) -> TCStrings { + fn as_ctype(arg: Self::RustType) -> Self { // emulate Vec::into_raw_parts(): // - disable dropping the Vec with ManuallyDrop // - extract ptr, len, and capacity using those methods - let (items, len, _capacity) = vec_into_raw_parts(self); + let (items, len, _capacity) = vec_into_raw_parts(arg); TCStrings { len, _capacity, @@ -55,10 +55,10 @@ impl Default for TCStrings { /// Free a TCStrings instance. The instance, and all TCStrings it contains, must not be used after /// this call. #[no_mangle] -pub extern "C" fn tc_strings_free<'a>(tctags: *mut TCStrings) { - debug_assert!(!tctags.is_null()); +pub extern "C" fn tc_strings_free<'a>(tcstrings: *mut TCStrings) { + debug_assert!(!tcstrings.is_null()); // SAFETY: - // - *tctags is a valid TCStrings (caller promises to treat it as read-only) - let tags = unsafe { Vec::take_from_arg(tctags, TCStrings::default()) }; - drop_pointer_array(tags); + // - *tcstrings is a valid TCStrings (caller promises to treat it as read-only) + let strings = unsafe { TCStrings::take_from_arg(tcstrings, TCStrings::default()) }; + drop_pointer_array(strings); } diff --git a/lib/src/task.rs b/lib/src/task.rs index fff4c490a..1a2bbaf6d 100644 --- a/lib/src/task.rs +++ b/lib/src/task.rs @@ -247,7 +247,7 @@ pub extern "C" fn tc_task_to_immut<'a>(task: *mut TCTask) { /// Get a task's UUID. #[no_mangle] pub extern "C" fn tc_task_get_uuid(task: *mut TCTask) -> TCUuid { - wrap(task, |task| task.get_uuid().return_val()) + wrap(task, |task| TCUuid::return_val(task.get_uuid())) } /// Get a task's status. @@ -331,7 +331,7 @@ pub extern "C" fn tc_task_get_tags<'a>(task: *mut TCTask) -> TCStrings { .expect("TCString::return_val() returned NULL") }) .collect(); - tcstrings.return_val() + TCStrings::return_val(tcstrings) }) } diff --git a/lib/src/traits.rs b/lib/src/traits.rs index 8011a46e4..df57d11c7 100644 --- a/lib/src/traits.rs +++ b/lib/src/traits.rs @@ -2,29 +2,30 @@ /// values are implicitly copyable, via C's struct assignment. /// /// The Rust and C types may differ, with from_ctype and as_ctype converting between them. +/// Implement this trait for the C type. pub(crate) trait PassByValue: Sized { - type CType; + type RustType; /// Convert a C value to a Rust value. /// /// # Safety /// - /// `arg` must be a valid CType. - unsafe fn from_ctype(arg: Self::CType) -> Self; + /// `self` must be a valid CType. + unsafe fn from_ctype(self) -> Self::RustType; /// Convert a Rust value to a C value. - fn as_ctype(self) -> Self::CType; + fn as_ctype(arg: Self::RustType) -> Self; /// Take a value from C as an argument. /// /// # Safety /// - /// `arg` must be a valid CType. This is typically ensured either by requiring that C + /// `self` must be a valid CType. This is typically ensured either by requiring that C /// code not modify it, or by defining the valid values in C comments. - unsafe fn from_arg(arg: Self::CType) -> Self { + unsafe fn from_arg(arg: Self) -> Self::RustType { // SAFETY: // - arg is a valid CType (promised by caller) - unsafe { Self::from_ctype(arg) } + unsafe { arg.from_ctype() } } /// Take a value from C as a pointer argument, replacing it with the given value. This is used @@ -33,7 +34,7 @@ pub(crate) trait PassByValue: Sized { /// # Safety /// /// `*arg` must be a valid CType, as with [`from_arg`]. - unsafe fn take_from_arg(arg: *mut Self::CType, mut replacement: Self::CType) -> Self { + unsafe fn take_from_arg(arg: *mut Self, mut replacement: Self) -> Self::RustType { // SAFETY: // - arg is valid (promised by caller) // - replacement is valid (guaranteed by Rust) @@ -44,8 +45,8 @@ pub(crate) trait PassByValue: Sized { } /// Return a value to C - fn return_val(self) -> Self::CType { - self.as_ctype() + fn return_val(arg: Self::RustType) -> Self { + Self::as_ctype(arg) } /// Return a value to C, via an "output parameter" @@ -54,12 +55,12 @@ pub(crate) trait PassByValue: Sized { /// /// `arg_out` must not be NULL and must be properly aligned and pointing to valid memory /// of the size of CType. - unsafe fn to_arg_out(self, arg_out: *mut Self::CType) { + unsafe fn to_arg_out(val: Self::RustType, arg_out: *mut Self) { debug_assert!(!arg_out.is_null()); // SAFETY: // - arg_out is not NULL (promised by caller, asserted) // - arg_out is properly aligned and points to valid memory (promised by caller) - unsafe { *arg_out = self.as_ctype() }; + unsafe { *arg_out = Self::as_ctype(val) }; } } diff --git a/lib/src/uuid.rs b/lib/src/uuid.rs index dfb084887..3c7751747 100644 --- a/lib/src/uuid.rs +++ b/lib/src/uuid.rs @@ -10,30 +10,30 @@ use taskchampion::Uuid; #[repr(C)] pub struct TCUuid([u8; 16]); -impl PassByValue for Uuid { - type CType = TCUuid; +impl PassByValue for TCUuid { + type RustType = Uuid; - unsafe fn from_ctype(arg: TCUuid) -> Self { + unsafe fn from_ctype(self) -> Self::RustType { // SAFETY: // - any 16-byte value is a valid Uuid - Uuid::from_bytes(arg.0) + Uuid::from_bytes(self.0) } - fn as_ctype(self) -> TCUuid { - TCUuid(*self.as_bytes()) + fn as_ctype(arg: Uuid) -> Self { + TCUuid(*arg.as_bytes()) } } /// Create a new, randomly-generated UUID. #[no_mangle] pub extern "C" fn tc_uuid_new_v4() -> TCUuid { - Uuid::new_v4().return_val() + TCUuid::return_val(Uuid::new_v4()) } /// Create a new UUID with the nil value. #[no_mangle] pub extern "C" fn tc_uuid_nil() -> TCUuid { - Uuid::nil().return_val() + TCUuid::return_val(Uuid::nil()) } // NOTE: this must be a simple constant so that cbindgen can evaluate it @@ -56,7 +56,7 @@ pub extern "C" fn tc_uuid_to_buf<'a>(tcuuid: TCUuid, buf: *mut libc::c_char) { }; // SAFETY: // - tcuuid is a valid TCUuid (all byte patterns are valid) - let uuid: Uuid = unsafe { Uuid::from_arg(tcuuid) }; + let uuid: Uuid = unsafe { TCUuid::from_arg(tcuuid) }; uuid.to_hyphenated().encode_lower(buf); } @@ -66,7 +66,7 @@ pub extern "C" fn tc_uuid_to_buf<'a>(tcuuid: TCUuid, buf: *mut libc::c_char) { pub extern "C" fn tc_uuid_to_str(tcuuid: TCUuid) -> *mut TCString<'static> { // SAFETY: // - tcuuid is a valid TCUuid (all byte patterns are valid) - let uuid: Uuid = unsafe { Uuid::from_arg(tcuuid) }; + let uuid: Uuid = unsafe { TCUuid::from_arg(tcuuid) }; let s = uuid.to_string(); // SAFETY: see TCString docstring unsafe { TCString::from(s).return_val() } @@ -85,7 +85,7 @@ pub extern "C" fn tc_uuid_from_str<'a>(s: *mut TCString, uuid_out: *mut TCUuid) // SAFETY: // - uuid_out is not NULL (promised by caller) // - alignment is not required - unsafe { u.to_arg_out(uuid_out) }; + unsafe { TCUuid::to_arg_out(u, uuid_out) }; return true; } } diff --git a/lib/taskchampion.h b/lib/taskchampion.h index 84c471b26..818fc166f 100644 --- a/lib/taskchampion.h +++ b/lib/taskchampion.h @@ -127,16 +127,13 @@ typedef struct TCUuid { } TCUuid; /** - * TCStrings represents a list of tags associated with a task. + * TCStrings represents a list of strings. * * The content of this struct must be treated as read-only. - * - * The lifetime of a TCStrings instance is independent of the task, and it - * will remain valid even if the task is freed. */ typedef struct TCStrings { /** - * number of tags in items + * number of strings in items */ size_t len; /** @@ -144,7 +141,7 @@ typedef struct TCStrings { */ size_t _capacity; /** - * strings representing each tag. these remain owned by the TCStrings instance and will be freed + * strings representing each string. these remain owned by the TCStrings instance and will be freed * by tc_strings_free. */ struct TCString *const *items; @@ -276,7 +273,7 @@ void tc_string_free(struct TCString *tcstring); * Free a TCStrings instance. The instance, and all TCStrings it contains, must not be used after * this call. */ -void tc_strings_free(struct TCStrings *tctags); +void tc_strings_free(struct TCStrings *tcstrings); /** * Convert an immutable task into a mutable task. @@ -349,13 +346,16 @@ bool tc_task_is_waiting(struct TCTask *task); bool tc_task_is_active(struct TCTask *task); /** - * Check if a task has the given tag. If the tag is invalid, this function will simply return - * false, with no error from `tc_task_error`. + * Check if a task has the given tag. If the tag is invalid, this function will return false, as + * that (invalid) tag is not present. No error will be reported via `tc_task_error`. */ bool tc_task_has_tag(struct TCTask *task, struct TCString *tag); /** - * Get the tags for the task. The task must not be NULL. + * Get the tags for the task. + * + * The caller must free the returned TCStrings instance. The TCStrings instance does not + * reference the task and the two may be freed in any order. */ struct TCStrings tc_task_get_tags(struct TCTask *task);