Re: [Libguestfs] [PATCH libnbd 2/2] generator: Change handling of Flags to be a true optional argument.
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
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)