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

[Richard W.M. Jones]

On Fri, Aug 09, 2019 at 08:31:22AM -0500, Eric Blake wrote:
> On 8/9/19 7:59 AM, Richard W.M. Jones wrote:
> > 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.

For interest the way we do this in libguestfs is inspired by Sun's
XView which used va_args like this:

  panel_item = xv_create(panel, PANEL_CHOICE_STACK,
    XV_WIDTH,             50,
    XV_HEIGHT,            25,
    PANEL_LABEL_X,        100,
    PANEL_LABEL_Y,        100,
    PANEL_LABEL_STRING,   "Open File",
    PANEL_CHOICE_STRINGS, "Append to file",
                          "Overwrite contents",NULL,
    NULL);

(Example from: ftp://ftp.ora.com/pub/openlook/vol7a.pdf page 23)

However that would break our existing API, and it seems like if you're
using C it's not a big deal to specify all the optional arguments,
particularly for libnbd which is a fairly simple API where we don't
expect to use optargs very much.

In contrast the most optargs used by a libguestfs API is 12 for
guestfs_add_drive_opts:

  http://libguestfs.org/guestfs.3.html#guestfs_add_drive_opts

so there (for libguestfs) it does make sense to have a way to supply a
subset from C.

> 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...

Indeed.

> > +    | (name, _) ->
> > +       failwithf "%s: optargs can only be empty list of [OFlags]" name
> 
> s/of/or/

Thanks - fixed in my copy.

> > @@ -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.

Yes that was wrong too, fixed in my copy.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW