Re: [Libguestfs] [libnbd PATCH 2/4] api: Rearrange flags argument to block_status
On 5/28/19 4:24 AM, Richard W.M. Jones wrote:
Patches 1 & 2 are fine, ACK. I have a separate comment about
patch 4
coming up.
In libguestfs we handle optional args by putting them in a
separate list (so instead of { args; ret } we have we have
{ args; optargs; ret }).
If we ever need more than just flags as an optarg, we'll have to think
about doing that. But for now, the simpler case of just treating Flags
as the only optional argument, and always last, seems to work.
This gets translated into a language like Python in the natural
way. However for C it's translated into a varargs list with
a rather complex system of flags, see:
http://libguestfs.org/guestfs.3.html#calls-with-optional-arguments
This has the advantage of supporting optional arguments that can be
nullable (or non-zero default value for ints) vs. not present.
On the other hand this may be overkill for libnbd. If the only
optional argument we support we care about is a non-zero int (or say a
non-nullable string where NULL = not present) then mixing the optional
args into the same list could be fine.
Do we have a good idea about what optional arguments apart from
non-zero ints we might need in future?
Off-hand, I'm not seeing enough cases that would demand such complexity.
Once we declare API stability, it may be trickier to revisit the
question, so hopefully we aren't painting ourselves into a corner by
limiting optional arguments to just flags.
Also, does Python really require optional arguments to be last in the
list? (OCaml can have them anywhere in the list)
Correct -
https://docs.python.org/2/reference/compound_stmts.html#function-definitions
"If a parameter has a default value, all following parameters must also
have a default value — this is a syntactic restriction that is not
expressed by the grammar." (still relevant in python 3)
--
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)