On 8/9/19 7:59 AM, Richard W.M. Jones wrote:
In libguestfs generator we have the concept of optargs which is different from plain arguments. These are mapped to optional arguments in languages that support them such as Python.
This commit adds a new concept of optargs. At the moment it is simply limited to handling the optional (in some bindings) flags parameter which is used to handle the NBD_CMD_FLAG_* flags.
If present, the old Flags parameter becomes OFlags and is moved into the optargs list.
For the OCaml generation this change simplifies things considerably as we no longer need the mapping from C arg to ocaml_arg (they are now the same).
In the libguestfs C bindings the handling of optargs is rather complex, and I don't intend to replicate that here. Instead they are just handled as non-optional arguments appearing after the normal arguments.
C can emulate optional arguments via va_args, but with the drawback that you have to provide some way at the callsite for the implementation to know which varargs to expect; it gets hairy fast. Keeping things as non-optional in libnbd C bindings seems reasonable enough. Another possibility is to generate n different function names (each adding one additional optarg) or even 2^n function names (one for each combination of which optional args are being provided); in that light, our existing nbd_aio_pread vs. nbd_aio_pread_callback could be viewed as two expansions with an optional completion Closure argument, where we generate 2 C bindings, but where other languages could have just a single entry point with an optional Closure argument. But that's obviously ideas for a future patch...
Note this commit does not change the API in any language. --- generator/generator | 505 +++++++++++++++++++++++--------------------- 1 file changed, 259 insertions(+), 246 deletions(-)
@@ -3157,17 +3169,14 @@ end = struct
(* Check the API definition. *) let () = - (* Flags must only appear once in the final argument position. *) + (* Currently optargs can only be [] or [OFlags]. This condition + * will be relaxed later when we support more optional arguments. + *)
...and you even mention that we might have more optional arguments in the future.
List.iter ( - fun (name, { args }) -> - let args = List.rev args in - match args with - | [] -> () - | Flags _ :: xs - | xs -> - if List.exists (function Flags _ -> true | _ -> false) xs then - failwithf "%s: Flags must appear in final argument position only" - name + function + | _, { optargs = [] } | _, { optargs = [OFlags _] } -> () + | (name, _) -> + failwithf "%s: optargs can only be empty list of [OFlags]" name
s/of/or/
@@ -3554,7 +3569,7 @@ let permitted_state_text permitted_states = *) let generate_lib_api_c () = (* Print the wrapper added around all API calls. *) - let rec print_wrapper (name, {args; ret; permitted_states; + let rec print_wrapper (name, {args; optargs; ret; permitted_states; is_locked; may_set_error}) =
Indentation looks odd to me, but that may be because I'm not familiar enough with OCaml indentation conventions. ACK with the typo fix. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org