[Libguestfs] [libnbd PATCH 04/10] generator: Add information about the lifetime of closures
Tage Johansson
tage.j.lists at posteo.net
Fri Jul 21 10:13:02 UTC 2023
On 7/19/2023 4:35 PM, Richard W.M. Jones wrote:
> On Wed, Jul 19, 2023 at 09:09:48AM +0000, Tage Johansson wrote:
>> Add a new field `cbkind` to the `closure` type in generator/API.ml*.
>> It tells how many times the closure may be invoked and for how long time
>> it might be used. More specifically, it can take any of these values:
>> - `CBOnceCommand`: The closure may only be called once and shall not
>> be called after the command is retired.
>> - `CBManyCommand`: The closure may be called any number of times but
>> not after the command is retired.
>> - `CBManyHandle`: The closure may be called any number of times before
>> the handle is destructed.
>> This information is needed in the Rust bindings for:
>> a) Knowing if the closure trait should be `FnMut` or `FnOnce`
>> (see <https://doc.rust-lang.org/std/ops/trait.FnOnce.html>).
>> b) Knowing for what lifetime the closure should be valid. A closure that
>> may be called after the function invokation has returned must live
>> for the `'static` lifetime. But static closures are inconveniant for
>> the user since they can't effectively borrow any local data. So it is
>> good if this restriction is relaxed when it is not needed.
>> ---
>> generator/API.ml | 11 +++++++++++
>> generator/API.mli | 13 +++++++++++++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/generator/API.ml b/generator/API.ml
>> index f90a6fa..086b2f9 100644
>> --- a/generator/API.ml
>> +++ b/generator/API.ml
>> @@ -77,6 +77,7 @@ and ret =
>> and closure = {
>> cbname : string;
>> cbargs : cbarg list;
>> + cbkind : cbkind;
> I'm dubious about the premise of this patch, but let's at least call
> it "cblifetime" since that's what it is expressing.
The difference in code for the user might be something like the following:
With only static lifetimes, a call to `opt_list` might look like this:
```Rust
use std::sync::{Arc, Mutex}; // Collect all exports in this list.
// Collect all exports in this list.
let exports = Arc::new(Mutex::new(Vec::new()));
let exports_clone = exports.clone();
let count = nbd.opt_list(move |name, _| {
exports_clone.lock().unwrap().push(name.to_owned());
0
})?;
let exports = Arc::into_inner(exports).unwrap().into_inner().unwrap();
assert_eq!(export.as_c_str(), expected);
```
And with custom lifetimes:
```Rust
// Collect all exports in this list.
let mut exports = Vec::new();
let count = nbd.opt_list(|name, _| {
exports.push(name.to_owned());
0
})?;
assert_eq!(exports.len(), count as usize);
```
Not only is the latter shorter and easier to read, it is also more
efficient. But it is not strictly necessary, and I can remove it if you
want.
You are correct though that `cbkind` is not a particularly good name.
`cblifetime` might be better, but actually it is describing two things:
Both the lifetime of the closure and how many times the closure might be
called. Perhaps it might be better to split it into two fields:
`cblifetime` and `cbcount`?
>> }
>> and cbarg =
>> | CBArrayAndLen of arg * string
>> @@ -87,6 +88,10 @@ and cbarg =
>> | CBString of string
>> | CBUInt of string
>> | CBUInt64 of string
>> +and cbkind =
>> +| CBOnceCommand
>> +| CBManyCommand
>> +| CBManyHandle
>> and enum = {
>> enum_prefix : string;
>> enums : (string * int) list
>> @@ -141,20 +146,24 @@ the handle from the NBD protocol handshake."
>> (* Closures. *)
>> let chunk_closure = {
>> cbname = "chunk";
>> + cbkind = CBManyCommand;
>> cbargs = [ CBBytesIn ("subbuf", "count");
>> CBUInt64 "offset"; CBUInt "status";
>> CBMutable (Int "error") ]
>> }
>> let completion_closure = {
>> cbname = "completion";
>> + cbkind = CBOnceCommand;
>> cbargs = [ CBMutable (Int "error") ]
>> }
>> let debug_closure = {
>> cbname = "debug";
>> + cbkind = CBManyHandle;
>> cbargs = [ CBString "context"; CBString "msg" ]
>> }
>> let extent_closure = {
>> cbname = "extent";
>> + cbkind = CBManyCommand;
>> cbargs = [ CBString "metacontext";
>> CBUInt64 "offset";
>> CBArrayAndLen (UInt32 "entries",
>> @@ -163,10 +172,12 @@ let extent_closure = {
>> }
>> let list_closure = {
>> cbname = "list";
>> + cbkind = CBManyCommand;
>> cbargs = [ CBString "name"; CBString "description" ]
>> }
>> let context_closure = {
>> cbname = "context";
>> + cbkind = CBManyCommand;
>> cbargs = [ CBString "name" ]
>> }
>> let all_closures = [ chunk_closure; completion_closure;
>> diff --git a/generator/API.mli b/generator/API.mli
>> index 361132d..73dc40a 100644
>> --- a/generator/API.mli
>> +++ b/generator/API.mli
>> @@ -94,6 +94,10 @@ and ret =
>> and closure = {
>> cbname : string; (** name of callback function *)
>> cbargs : cbarg list; (** all closures return int for now *)
>> + (** Wether the closure should be used once or many times and wether it might
> wether -> whether
>
> Rich.
>
>> + be used only until the command is completed or until the handle is
>> + destructed. *)
>> + cbkind : cbkind;
>> }
>> and cbarg =
>> | CBArrayAndLen of arg * string (** array + number of entries *)
>> @@ -104,6 +108,15 @@ and cbarg =
>> | CBString of string (** like String *)
>> | CBUInt of string (** like UInt *)
>> | CBUInt64 of string (** like UInt64 *)
>> +and cbkind =
>> +| CBOnceCommand (** The closure will be used 0 or 1 time if the aio_* call
>> + returned a error and exactly once if the call succeeded.
>> + The closure may only be called before the command
>> + is retired. (E.g., completion callback.) *)
>> +| CBManyCommand (** The closure may be used any number of times but only
>> + before the command is retired. (E.g., list callback.) *)
>> +| CBManyHandle (** The closure may be used any number of times for as long
>> + as the handle exists. (E.g., debug callback.) *)
>> and enum = {
>> enum_prefix : string; (** prefix of each enum variant *)
>> enums : (string * int) list (** enum names and their values in C *)
>> --
>> 2.41.0
>>
>> _______________________________________________
>> Libguestfs mailing list
>> Libguestfs at redhat.com
>> https://listman.redhat.com/mailman/listinfo/libguestfs
More information about the Libguestfs
mailing list