[Libguestfs] [PATCH libnbd 1/2] generator: Handle closure args (cbargs) specially.

[Richard W.M. Jones]

On Fri, Aug 09, 2019 at 08:13:32AM -0500, Eric Blake wrote:
> On 8/9/19 7:59 AM, Richard W.M. Jones wrote:
> > If we accept that callbacks will never handle the full range of
> > parameters then we can simplify the generator quite a bit by using a
> > special type for closure args vs normal method args.
> 
> Nice.
> 
> > 
> > This removes many asserts and quite a bit of unreachable code
> > (eg. Python code for handling ArrayAndLen in normal methods that was
> > never used).
> 
> Or having a closure arg within a callback.
> 
> > 
> > The output of the generator after this commit should be identical.
> > 
> > It's possible to go a little further if we wanted: CBArrayAndLen is
> > only ever used for ‘uint32_t’ arrays.  CBMutable is only ever used for
> > ‘int*’.  We could make CB* types which only handle those cases.
> > ---
> >  generator/generator | 428 +++++++++++++++++++-------------------------
> >  1 file changed, 186 insertions(+), 242 deletions(-)
> > 
> 
> Relatively large, but does look like a simplification.
> 
> 
> > +and cbarg =
> > +| CBArrayAndLen of arg * string (* array + number of entries *)
> > +| CBBytesIn of string * string (* like BytesIn *)
> > +| CBInt of string          (* like Int *)
> > +| CBInt64 of string        (* like Int64 *)
> > +| CBMutable of arg         (* mutable argument, eg. int* *)
> 
> Should we use 'of cbarg' instead of 'of arg' for CBArrayAndLen and
> CBMutable?  Or, as you said above, we could make it a specific type
> instead of trying to keep a generic wrapper,

I don't think "of cbarg" actually works because there's no unsigned
int32 type there.  We could as I mention in the commit change this to
"CBUInt32ArrayAndLen", "CBMutableInt", then the arg parameter would
truly disappear.

> with the cost that if we
> add new callbacks down the road we may want to reintroduce the generic
> wrapper at that time instead of continuing to add one type per use.

Yes, I think at the moment we can wait and see how the API evolves.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v