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/