[Libguestfs] [libnbd PATCH 1/3] generator: Introduce REnum/RFlags return types

[Richard W.M. Jones]

On Mon, Sep 07, 2020 at 09:38:15AM -0500, Eric Blake wrote:
> On 9/7/20 9:13 AM, Richard W.M. Jones wrote:
> 
> >>+++ b/generator/C.ml
> >>@@ -66,15 +66,15 @@ let errcode_of_ret =
> >>    function
> >>    | RBool | RErr | RFd | RInt | RInt64 | RCookie -> Some "-1"
> >>    | RStaticString | RString -> Some "NULL"
> >>-  | RUInt -> None (* errors not possible *)
> >>+  | RUInt | REnum (_) | RFlags (_) -> None (* errors not possible *)
> >>
> >>  let type_of_ret =
> >>    function
> >>-  | RBool | RErr | RFd | RInt -> "int"
> >>+  | RBool | RErr | RFd | RInt | REnum (_) -> "int"
> >
> >This is good because Enum is passed as int.
> >
> >>[...]
> >
> >You used unsigned for RFlags, but shouldn't it be this instead?
> >
> >+  | RFlags (_) -> "uint32_t"
> 
> Hmm.  For Flags as input, we take uint32_t, but for
> get_handshake_flags which used to return RUint, we returned
> unsigned; as written, my patch preserved generated code in its
> entirety.  Changing it to return uint32_t makes sense, but is a
> minor API change.

Oh I see, good point.

I think although technically it might not be an ABI change,
best to leave it as you wrote it.

Rich.

> Fortunately, it would not be an ABI change on any
> of the platforms we compile on.  So I can go ahead and return the
> smarter type unless we have a strong reason not to (the API change
> might affect anyone that was storing a function pointer to
> nbd_get_handshake_flags, and more so in C++ than C).
> 
> >
> >The rest of the patch looks fine.
> >
> >Rich.
> >
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/