On 8/15/2023 11:07 AM, Richard W.M. Jones wrote:
So what do I think about the patch series as a whole ... (in
particular, the patches I didn't add Reviewed-by tags to).
It would be much nicer IMHO if we didn't have to define callback
lifetimes in this way, since they were not intended to be classified
into async_kind / cblifetime / cbcount, and this might limit our
options for new ABIs in future.
I see two ways to go here:
(1) (Easier for now, problems in future) Rename async_kind, cblifetime
and cbcount as rust_async_kind, rust_cblifetime, rust_cbcount, which
would in some sense limit the scope of getting these right to the Rust
bindings.
This defers the pain til later (maybe never, if we never added an ABI
which didn't satisfy these constraints).
(2) (Harder for now, no problems in future) Use a reference count in
the Rust bindings, which is how the other bindings work. It makes the
Rust bindings more awkward to use, but does more accurately express
he actual intention of the API.
Discuss ...
I want to emphasize the different purposes of async_kind, cblifetime and
cbcount:
- async_kind: Tells what calls are asynchronous or not and how they
mark completion. It has nothing to do with reference counting and is
required for the asynchronous Rust bindings to work.
- cblifetime: Tells for how long a closure might be used. Without this
the user would need to use a reference counter (Arc) and a Mutex. The
API would be a bit more verbose/awkward to use, but it would
definitely be possible.
- cbcount: Tells for how many times the closure might be called. Without
this, all closures would be FnMut, including completion callbacks.
I don't think it would hurt too much to remove this.
So unless someone comes up with a better solution, I think we have to
keep the async_kind field. Although it would make sense to rename it to
rust_async_kind.
Personally I think the other two makes sense as well, but they can
definitely be removed if that's what the community wants.
--
Best regards,
Tage
Rich.
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
