Re: [Libguestfs] [libnbd PATCH] api: Generate flag values as unsigned

C says that performing bitwise math on ints can lead to corner-case undefined values. When we document something as being usable as a flag with bitwise-OR and similar, we should ensure the values are unsigned, to avoid those corner-cases in C. Even our own codebase was potentially falling foul of strict C rules, with code like tests/errors-server-invalid-offset.c: uint32_t strict; strict = nbd_get_strict_mode (nbd) & ~LIBNBD_STRICT_BOUNDS; In theory, this is an API change that might be detectable by someone trying to probe which integer promotions occur on a given constant. In practice, no one writes code that intentionally tries to exploit whether a flag is signed or unsigned, and none of our values were large enough to worry about sign-extension effects that occur when flipping the sign bit of a signed value (we tend to target machines with twos-complement signed values where the behavior is sane despite being undefined by C), so this patch is therefore not an ABI change. Generated code changes as follows: | --- include/libnbd.h.bak 2022-11-11 11:15:54.929686924 -0600 | +++ include/libnbd.h 2022-11-11 11:15:56.652698919 -0600 | @@ -69,32 +69,32 @@ | #define LIBNBD_SIZE_MAXIMUM 2 | #define LIBNBD_SIZE_PAYLOAD 3 | | -#define LIBNBD_CMD_FLAG_FUA 0x01 | -#define LIBNBD_CMD_FLAG_NO_HOLE 0x02 | ... | -#define LIBNBD_ALLOW_TRANSPORT_VSOCK 0x04 | -#define LIBNBD_ALLOW_TRANSPORT_MASK 0x07 | +#define LIBNBD_CMD_FLAG_FUA 0x01U | +#define LIBNBD_CMD_FLAG_NO_HOLE 0x02U | ... | +#define LIBNBD_ALLOW_TRANSPORT_VSOCK 0x04U | +#define LIBNBD_ALLOW_TRANSPORT_MASK 0x07U Thanks: Laszlo Ersek <lersek(a)redhat.com&gt; --- I'll push this along with the LIBNBD_STRICT_PAYLOAD series. generator/C.ml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/generator/C.ml b/generator/C.ml index cfa2964c..f9171996 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -437,11 +437,11 @@ let List.iter ( fun (flag, i) -> let flag = sprintf "LIBNBD_%s_%s" flag_prefix flag in - pr "#define %-40s 0x%02x\n" flag i; + pr "#define %-40s 0x%02xU\n" flag i; mask := !mask lor i ) flags; let flag = sprintf "LIBNBD_%s_MASK" flag_prefix in - pr "#define %-40s 0x%02x\n" flag !mask; + pr "#define %-40s 0x%02xU\n" flag !mask; pr "\n" ) all_flags; List.iter ( --