On 9/7/20 9:52 AM, Richard W.M. Jones wrote:
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.
Not only is it not an ABI change, but we got lucky: no one has ever compiled libnbd on a platform where it would be an API change. Note that pre-patch, our generated lib/unlocked.h had 'unsigned nbd_get_handshake_flags' but our hand-written lib/handle.c had 'uint32_t nbd_get_handshake_flags'. On every platform where libnbd compiles, the two types are synonymous, so the API change factor is a non-issue (platforms where it might not be the same could be a 32-bit platform where 'uint32_t' is 'long', but no one has reported trying to build libnbd on such a system). So I'll go ahead and make the change, with the commit message giving this additional justification. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org