Hi
On Fri, Sep 11, 2020 at 5:08 PM Paolo Bonzini <pbonz...@redhat.com> wrote:
> 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)
