On 11/09/20 12:24, Paolo Bonzini wrote:
>>
>> - from_qemu_none(ptr: *const sys::P) -> T
>> Return a Rust type T for a const ffi pointer P.
>>
>> - from_qemu_full(ptr: *mut sys::P) -> T
>> Return a Rust type T for a ffi pointer P, taking ownership.
>>
>> - T::to_qemu_none() -> Stash<P>
>> Returns a borrowed ffi pointer P (using a Stash to destroy "glue"
>> storage data, if any).
>>
>> - T::to_qemu_full() -> P
>> Returns a ffi pointer P. (P resources are leaked/passed to C/ffi)
>
> I know these come from glib-rs, but still the names are awful. :)
After studying a bit I managed to give a rational explanation of the
above gut feeling. Conversions in Rust usually follow this naming
convention:
Name Type Price
-------- -------------------- -----------
as_*, borrow Borrowed -> Borrowed Cheap
to_*, constructor Borrowed -> Owned Expensive
from, into_* Owned -> Owned Any
and we have
from_qemu_none Borrowed -> Owned
to_qemu_none Borrowed -> Borrowed
from_qemu_full Owned -> Owned
to_qemu_full Owned -> Owned
So
- from_qemu_none should be a "to_*" or "constructor" conversion (I used
new_from_foreign)
new_ prefix is not very rusty.
from_raw() is common, and takes ownership.
from_ptr() (as in CStr) could be a candidate (although it wouldn't have the same lifetime requirements)
- to_qemu_none, even though in some cases it can be expensive, should be
an "as_*" conversion (as_foreign).
as_ptr(), but since it's more complicated than just a pointer due to temporary storage, it's not the best fit either...
- from_qemu_full and to_qemu_full should be "from" or "into_*"
ok
to_qemu_full() could also be split into a more expensive but flexible
"to" variant and the cheaper "into" variant. Just for the sake of
example, I updated my demo by replacing the IntoRaw trait with these two:
trait ToForeign<T> {
fn to_foreign(&self) -> *mut T;
}
trait IntoForeign<T> {
fn into_foreign(self) -> *mut T;
}
where the example implementations show the different cost quite clearly:
impl ToForeign<c_char> for String {
fn to_foreign(&self) -> *mut c_char {
unsafe { libc::strndup(self.as_ptr() as *const c_char,
self.len() as size_t) }
}
}
impl IntoForeign<c_char> for String {
fn into_foreign(self) -> *mut c_char {
let ptr = self.as_ptr();
forget(self);
ptr as *mut _
}
}
You corrected the \0-ending in the git tree. However, the memory allocator (or the stack) may not be compatible with the one used in C.
As for the general comparison with glib-rs traits, it's hard for me to say upfront the pros and cons, I would need to modify this PoC for example. But I must say I feel quite comfortable with the glib approach. It would be nice to have some feedback from glib-rs maintainers about your proposal.
(my gut feeling is that we would be better sticking with something close to glib, as it is likely we will end up using glib-rs at some point, and having similar patterns should help each other)