Richard W.M. Jones
2023-Jul-19 14:35 UTC
[Libguestfs] [libnbd PATCH 04/10] generator: Add information about the lifetime of closures
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.> } > 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 mightwether -> 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-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html
Tage Johansson
2023-Jul-21 10:13 UTC
[Libguestfs] [libnbd PATCH 04/10] generator: Add information about the lifetime of closures
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