[Libguestfs] [PATCH libnbd 2/2] generator: Change handling of Flags to be a true optional argument.

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20190809/e909dba9/attachment.sig>