search for: oclosure

...ons(-) diff --git a/generator/generator b/generator/generator index 01da1c3..3add9a4 100755 --- a/generator/generator +++ b/generator/generator @@ -867,6 +867,7 @@ and arg = | UInt32 of string (* 32 bit unsigned int *) | UInt64 of string (* 64 bit unsigned int *) and optarg = +| OClosure of closure (* optional closure *) | OFlags of string * flags (* optional flags, uint32_t in C *) and ret = | RBool (* return a boolean, or error *) @@ -3184,16 +3185,6 @@ end = struct (* Check the API definition. *) let () = - (* Currently optargs can only be [] or [...

...ons(-) diff --git a/generator/generator b/generator/generator index 8cf95b6..8f15786 100755 --- a/generator/generator +++ b/generator/generator @@ -867,6 +867,7 @@ and arg = | UInt32 of string (* 32 bit unsigned int *) | UInt64 of string (* 64 bit unsigned int *) and optarg = +| OClosure of closure (* optional closure *) | OFlags of string * flags (* optional flags, uint32_t in C *) and ret = | RBool (* return a boolean, or error *) @@ -3184,16 +3185,6 @@ end = struct (* Check the API definition. *) let () = - (* Currently optargs can only be [] or [...

...c | 45 -------------------------------------- 8 files changed, 67 insertions(+), 96 deletions(-) diff --git a/generator/API.ml b/generator/API.ml index 42eeac0..b212e95 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -55,7 +55,7 @@ and arg = | UInt64 of string and optarg = | OClosure of closure -| OFlags of string * flags +| OFlags of string * flags * string list option and ret = | RBool | RStaticString @@ -1485,7 +1485,11 @@ Future NBD extensions may result in additional C<size_type> values. "pread", { default_call with args = [ BytesOut ("...

v1 was here: https://www.redhat.com/archives/libguestfs/2019-August/msg00168.html I pushed uncontroversial patches 1-4 v2: - The implementation of OClosure (new patch 1) in Python is fixed. - Patch 2 (old patch 5) is unchanged. - I added a new API for removing debug callbacks. I think this approach has some advantages over using OClosure. - I didn't yet do any work on changing the aio_*_callback functions to aio_*. Rich.

On 8/13/19 5:06 AM, Richard W.M. Jones wrote: > An optional Closure parameter, but otherwise works the same way as > Closure. > @@ -3778,6 +3777,7 @@ let generate_lib_api_c () = > ) args; > List.iter ( > function > + | OClosure { cbname } -> pr ", %s_callback ? \"<fun>\" : \"NULL\"" cbname Well, it also permits a NULL fn pointer. > @@ -4383,6 +4387,16 @@ let print_python_binding name { args; optargs; ret; may_set_error } = > ) args; > List.iter ( > function &...

...[ BytesPersistOut ("buf", "count"); UInt64 "offset"; - Closure completion_closure ]; - optargs = [ OFlags ("flags", cmd_flags) ]; + args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; + optargs = [ OClosure completion_closure; OFlags ("flags", cmd_flags) ]; ret = RInt64; permitted_states = [ Connected ]; shortdesc = "read from the NBD server, with callback on completion"; @@ -1874,9 +1873,8 @@ documented in C<nbd_pread_structured>."; "aio_pread_str...

...[ BytesPersistOut ("buf", "count"); UInt64 "offset"; - Closure completion_closure ]; - optargs = [ OFlags ("flags", cmd_flags) ]; + args = [ BytesPersistOut ("buf", "count"); UInt64 "offset" ]; + optargs = [ OClosure completion_closure; OFlags ("flags", cmd_flags) ]; ret = RInt64; permitted_states = [ Connected ]; shortdesc = "read from the NBD server, with callback on completion"; @@ -1874,9 +1873,8 @@ documented in C<nbd_pread_structured>."; "aio_pread_str...

...ameter, but otherwise works the same way as > Closure. > --- > generator/generator | 57 ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 46 insertions(+), 11 deletions(-) Do we need to edit the generated documentation strings to make it obvious that passing NULL for an OClosure function is supported (and/or the generated string for Closure that the function must not be NULL)? -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

...generator/generator | 57 ++++++++++++++++++++++++++++++++++++--------- > 1 file changed, 46 insertions(+), 11 deletions(-) > > @@ -4394,6 +4399,16 @@ let print_python_binding name { args; optargs; ret; may_set_error } = > ) args; > List.iter ( > function > + | OClosure { cbname } -> > + pr " if (%s_user_data != Py_None) {\n" cbname; > + pr " /* Increment refcount since pointer may be saved by libnbd. */\n"; > + pr " Py_INCREF (%s_user_data);\n" cbname; > + pr " if (!PyCallable_Che...

On Mon, Aug 12, 2019 at 11:00:01PM +0100, Richard W.M. Jones wrote: > On Mon, Aug 12, 2019 at 01:53:56PM -0500, Eric Blake wrote: > > We then have the design question of whether to make an OClosure type, > > where C has two functions nbd_aio_pread and nbd_aio_pread_callback for > > convenience, but where other languages have only a single nbd.aio_pread > > where the callback parameter is optional (the Closure type for > > pread_structured chunk and for block_status ext...

Patches 1-4 are basically uncontroversial, straightforward refactoring and IMHO we should just push them. Possibly 1-3 should be squashed together, but I posted them separately so they are easier to review. Patches 5 and 6 together implement OClosure. Patch 5 adds the feature and is simple to understand. Patch 6 changes the Closure completion callbacks into OClosure, but because it doesn't change the position within the list of arguments the C API doesn't change at all. The Python API also doesn't change for the same reason. (Th...

...8/13/19 5:06 AM, Richard W.M. Jones wrote: > > An optional Closure parameter, but otherwise works the same way as > > Closure. > > > @@ -3778,6 +3777,7 @@ let generate_lib_api_c () = > > ) args; > > List.iter ( > > function > > + | OClosure { cbname } -> pr ", %s_callback ? \"<fun>\" : \"NULL\"" cbname > > Well, it also permits a NULL fn pointer. This is right isn't it? > > @@ -4383,6 +4387,16 @@ let print_python_binding name { args; optargs; ret; may_set_error } = > >...

...e way as > > Closure. > > --- > > generator/generator | 57 ++++++++++++++++++++++++++++++++++++--------- > > 1 file changed, 46 insertions(+), 11 deletions(-) > > Do we need to edit the generated documentation strings to make it > obvious that passing NULL for an OClosure function is supported (and/or > the generated string for Closure that the function must not be NULL)? We should probably add that. It would apply to other parameters as well, eg. saying that String (const char *) cannot be NULL. Could be one for another patch. Rich. -- Richard Jones, Virtu...

In v2: - now based on my proposal to add LIBNBD_SHUTDOWN_IMMEDIATE - four flags instead of two: STRICT_FLAGS is new (patch 4), and STRICT_BOUNDS is separate from STRICT_ZERO_SIZE (patch 5) - various refactorings for more shared code and less duplication Eric Blake (5): api: Add xxx_MASK constant for each Flags type generator: Refactor filtering of accepted OFlags api: Add

...nnect_command(["true",1])' h.nbd_pread was leaking a read buffer on every single synchronous read. My previous patch to address closure callbacks had a bug in h.nbd_pread_structured and similar: if the allocation for Closure fails, we hit 'goto err' prior to initializing the OClosure pointers, which could lead to freeing garbage. Similarly, when messing with non-callable for either the Closure or the OClosure, there was enough inaccuracy in python reference counting to cause a leak or double-free. We were not consistently checking for python function failures (such as when a...

...f a callback is "null" or not, and set it to null. */ > -#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL) > +#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL && (cb).free == NULL) Semantic change. In generator, you used CALLBACK_IS_NULL() for both Closure and OClosure. For OClosure, the new semantics are still correct. But for Closure, we now no longer return EFAULT when the callback itself is missing but a .free was provided. This changes pread_structured and block_status to accept NULL for the callback; which is probably not a good idea. In short, there ar...

...r nbd.aio_pread_callback both need to use the C nbd_aio_pread_callback, for the sake of the completion's free_cb() to do the necessary refcount cleanup on the buf, at which point we no longer need a separate registration of free callbacks. We then have the design question of whether to make an OClosure type, where C has two functions nbd_aio_pread and nbd_aio_pread_callback for convenience, but where other languages have only a single nbd.aio_pread where the callback parameter is optional (the Closure type for pread_structured chunk and for block_status extent will still be mandatory; it is only...

...pr " nbd_%s_callback %s = { .callback = %s_wrapper,\n" cbname cbname cbname; pr " .user_data = %s_user_data,\n" cbname; @@ -316,7 +317,7 @@ let print_python_binding name { args; optargs; ret; may_set_error } = function | OClosure { cbname } -> pr " struct user_data *%s_user_data = alloc_user_data ();\n" cbname; - pr " if (%s_user_data == NULL) return NULL;\n" cbname; + pr " if (%s_user_data == NULL) goto out;\n" cbname; pr " nbd_%s_callback %s = { .callba...

On Mon, Aug 12, 2019 at 01:53:56PM -0500, Eric Blake wrote: > We then have the design question of whether to make an OClosure type, > where C has two functions nbd_aio_pread and nbd_aio_pread_callback for > convenience, but where other languages have only a single nbd.aio_pread > where the callback parameter is optional (the Closure type for > pread_structured chunk and for block_status extent will still be &g...

...t;string" + | SockAddrAndLen _ -> "string" + | String _ -> "string" + | StringList _ -> "[]string" + | UInt _ -> "uint" + | UInt32 _ -> "uint32" + | UInt64 _ -> "uint64" + +let go_name_of_optarg = function + | OClosure { cbname } -> String.capitalize_ascii cbname + | OFlags (n, _) -> String.capitalize_ascii n + +(* For consistency every method will return (type, error) where + * type is defined by this function. However some functions + * never really have a type (RErr) and others never really have + * an...