Re: [Libguestfs] [PATCH libnbd 2/2] generator: Change handling of Flags to be a true optional argument.
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