Re: [Libguestfs] [libnbd PATCH v2 1/5] api: Add xxx_MASK constant for each Flags type

Document and make it easier for applications to determine which bitmask values were supported at compile time. Also, generating bitmask values in hex tends to be more legible than in decimal, as it makes it obvious that the values are intended for bitwise operations. And it doesn't hurt that this reduces churn in some of the tests if new bits are added.

docs/libnbd.pod | 7 +++++++ generator/API.ml | 13 +++++++++++-- generator/C.ml | 13 ++++++++----- generator/GoLang.ml | 8 ++++++-- generator/OCaml.ml | 9 +++++++++ generator/Python.ml | 5 ++++- lib/handle.c | 5 ++--- python/t/110-defaults.py | 3 +-- python/t/120-set-non-defaults.py | 5 ++--- ocaml/tests/test_110_defaults.ml | 3 +-- ocaml/tests/test_120_set_non_defaults.ml | 3 +-- tests/errors.c | 9 +++------ .../libnbd/libnbd_110_defaults_test.go | 2 +- .../libnbd/libnbd_120_set_non_defaults_test.go | 4 ++-- 14 files changed, 58 insertions(+), 31 deletions(-) diff --git a/docs/libnbd.pod b/docs/libnbd.pod index f2ba3bb..d8dffea 100644 --- a/docs/libnbd.pod +++ b/docs/libnbd.pod @@ -520,6 +520,13 @@ To get the size of the export in bytes, use L<nbd_get_size(3)>: You can read and write data from the NBD server using L<nbd_pread(3)> and L<nbd_pwrite(3)> or their asynchronous equivalents. +All data commands support a C<flags> argument (mandatory in C, but +optional in languages where it can default to 0). For convenience, +the constant C<LIBNBD_CMD_FLAG_MASK> is defined with the set of flags +currently recognized by libnbd, where future NBD protocol extensions +may result in additional flags being supported; but in general, +specific data commands only accept a subset of known flags. + Some servers also support: =over 4 diff --git a/generator/API.ml b/generator/API.ml index cc1a7ba..42eeac0 100644 --- a/generator/API.ml +++ b/generator/API.ml @@ -679,6 +679,8 @@ fewer all-zero padding bytes over the connection. =back +For convenience, the constant C<LIBNBD_HANDSHAKE_FLAG_MASK> is +available to describe all flags supported by this build of libnbd. Future NBD extensions may add further flags, which in turn may be enabled by default in newer libnbd. As such, when attempting to disable only one specific bit, it is wiser to first call @@ -895,7 +897,11 @@ ORed together: =item C<LIBNBD_ALLOW_TRANSPORT_VSOCK> -=back"; +=back + +For convenience, the constant C<LIBNBD_ALLOW_TRANSPORT_MASK> is +available to describe all transports recognized by this build of +libnbd. A future version of the library may add new flags."; see_also = [Link "connect_uri"; Link "set_uri_allow_tls"]; }; @@ -1629,7 +1635,10 @@ issuing those commands before informing the server of the intent to disconnect. =back -"; + +For convenience, the constant C<LIBNBD_SHUTDOWN_MASK> is available +to describe all shutdown flags recognized by this build of libnbd. +A future version of the library may add new flags."; see_also = [Link "close"; Link "aio_disconnect"]; example = Some "examples/reads-and-writes.c"; }; diff --git a/generator/C.ml b/generator/C.ml index c9f0ff4..4d4958d 100644 --- a/generator/C.ml +++ b/generator/C.ml @@ -349,11 +349,15 @@ let generate_include_libnbd_h () = ) all_enums; List.iter ( fun { flag_prefix; flags } -> + let mask = ref 0 in List.iter ( fun (flag, i) -> let flag = sprintf "LIBNBD_%s_%s" flag_prefix flag in - pr "#define %-40s %d\n" flag i + pr "#define %-40s 0x%02x\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 "\n" ) all_flags; List.iter ( @@ -490,13 +494,12 @@ let generate_lib_api_c () = ); (* Check parameters are valid. *) - let print_flags_check n { flag_prefix; flags } = + let print_flags_check n { flag_prefix } = let value = match errcode with | Some value -> value | None -> assert false in - let mask = List.fold_left (lor) 0 (List.map snd flags) in - pr " if (unlikely ((%s & ~%d) != 0)) {\n" n mask; - pr " set_error (EINVAL, \"%%s: invalid value for flag: %%d\",\n"; + pr " if (unlikely ((%s & ~LIBNBD_%s_MASK) != 0)) {\n" n flag_prefix; + pr " set_error (EINVAL, \"%%s: invalid value for flag: 0x%%x\",\n"; pr " \"%s\", %s);\n" n n; pr " ret = %s;\n" value; pr " goto out;\n"; diff --git a/generator/GoLang.ml b/generator/GoLang.ml index 359c7d0..f07b074 100644 --- a/generator/GoLang.ml +++ b/generator/GoLang.ml @@ -447,12 +447,16 @@ import ( "; List.iter ( fun { flag_prefix; flags } -> - pr "type %s uint32\n" (camel_case flag_prefix); + let flag_type = camel_case flag_prefix in + let mask = ref 0 in + pr "type %s uint32\n" flag_type; pr "const (\n"; List.iter ( fun (flag, v) -> - pr " %s_%s = %s(%d)\n" flag_prefix flag (camel_case flag_prefix) v + pr " %s_%s = %s(0x%02x)\n" flag_prefix flag flag_type v; + mask := !mask lor v ) flags; + pr " %s_MASK = %s(0x%02x)\n" flag_prefix flag_type !mask; pr ")\n"; pr "\n" ) all_flags; diff --git a/generator/OCaml.ml b/generator/OCaml.ml index 4bcd450..43b3679 100644 --- a/generator/OCaml.ml +++ b/generator/OCaml.ml @@ -164,6 +164,8 @@ type cookie = int64 pr " | %s\n" flag ) flags; pr " | UNKNOWN of int\n"; + pr "\n"; + pr " val mask : t list\n"; pr "end\n"; pr "\n" ) all_flags; @@ -269,6 +271,13 @@ let () = pr " | %s\n" flag ) flags; pr " | UNKNOWN of int\n"; + pr "\n"; + pr " let mask = [\n"; + List.iter ( + fun (flag, _) -> + pr " %s;\n" flag + ) flags; + pr " ]\n"; pr "end\n"; pr "\n" ) all_flags; diff --git a/generator/Python.ml b/generator/Python.ml index 71368c6..fd09eae 100644 --- a/generator/Python.ml +++ b/generator/Python.ml @@ -696,11 +696,14 @@ Error.__str__ = _str ) all_enums; List.iter ( fun { flag_prefix; flags } -> + let mask = ref 0 in List.iter ( fun (flag, i) -> let flag = sprintf "%s_%s" flag_prefix flag in - pr "%s = %d\n" flag i + pr "%s = 0x%02x\n" flag i; + mask := !mask lor i ) flags; + pr "%s_MASK = 0x%02x\n" flag_prefix !mask; pr "\n" ) all_flags; List.iter (fun (n, i) -> pr "%s = %d\n" n i) constants; diff --git a/lib/handle.c b/lib/handle.c index 7987d59..4d26842 100644 --- a/lib/handle.c +++ b/lib/handle.c @@ -65,12 +65,11 @@ nbd_create (void) h->tls_verify_peer = true; h->request_sr = true; - h->uri_allow_transports = (uint32_t) -1; + h->uri_allow_transports = LIBNBD_ALLOW_TRANSPORT_MASK; h->uri_allow_tls = LIBNBD_TLS_ALLOW; h->uri_allow_local_file = false; - h->gflags = (LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE | - LIBNBD_HANDSHAKE_FLAG_NO_ZEROES); + h->gflags = LIBNBD_HANDSHAKE_FLAG_MASK; s = getenv ("LIBNBD_DEBUG"); h->debug = s && strcmp (s, "1") == 0; diff --git a/python/t/110-defaults.py b/python/t/110-defaults.py index 47c1d02..fb961cf 100644 --- a/python/t/110-defaults.py +++ b/python/t/110-defaults.py @@ -22,6 +22,5 @@ assert h.get_export_name() == "" assert h.get_full_info() is False assert h.get_tls() == nbd.TLS_DISABLE assert h.get_request_structured_replies() is True -assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE | - nbd.HANDSHAKE_FLAG_NO_ZEROES) +assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK assert h.get_opt_mode() is False diff --git a/python/t/120-set-non-defaults.py b/python/t/120-set-non-defaults.py index 3963021..3da0c23 100644 --- a/python/t/120-set-non-defaults.py +++ b/python/t/120-set-non-defaults.py @@ -34,11 +34,10 @@ if h.supports_tls(): h.set_request_structured_replies(False) assert h.get_request_structured_replies() is False try: - h.set_handshake_flags(nbd.HANDSHAKE_FLAG_NO_ZEROES << 1) + h.set_handshake_flags(nbd.HANDSHAKE_FLAG_MASK + 1) assert False except nbd.Error: - assert h.get_handshake_flags() == (nbd.HANDSHAKE_FLAG_NO_ZEROES | - nbd.HANDSHAKE_FLAG_FIXED_NEWSTYLE) + assert h.get_handshake_flags() == nbd.HANDSHAKE_FLAG_MASK h.set_handshake_flags(0) assert h.get_handshake_flags() == 0 h.set_opt_mode(True) diff --git a/ocaml/tests/test_110_defaults.ml b/ocaml/tests/test_110_defaults.ml index 54f2cbc..f5886fc 100644 --- a/ocaml/tests/test_110_defaults.ml +++ b/ocaml/tests/test_110_defaults.ml @@ -28,8 +28,7 @@ let () = let sr = NBD.get_request_structured_replies nbd in assert (sr = true); let flags = NBD.get_handshake_flags nbd in - assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE; - NBD.HANDSHAKE_FLAG.NO_ZEROES ]); + assert (flags = NBD.HANDSHAKE_FLAG.mask); let opt = NBD.get_opt_mode nbd in assert (opt = false) diff --git a/ocaml/tests/test_120_set_non_defaults.ml b/ocaml/tests/test_120_set_non_defaults.ml index 79fe184..b660e5d 100644 --- a/ocaml/tests/test_120_set_non_defaults.ml +++ b/ocaml/tests/test_120_set_non_defaults.ml @@ -46,8 +46,7 @@ let () = with NBD.Error _ -> (); let flags = NBD.get_handshake_flags nbd in - assert (flags = [ NBD.HANDSHAKE_FLAG.FIXED_NEWSTYLE; - NBD.HANDSHAKE_FLAG.NO_ZEROES ]); + assert (flags = NBD.HANDSHAKE_FLAG.mask); NBD.set_handshake_flags nbd []; let flags = NBD.get_handshake_flags nbd in assert (flags = []); diff --git a/tests/errors.c b/tests/errors.c index 906c7da..8166429 100644 --- a/tests/errors.c +++ b/tests/errors.c @@ -86,7 +86,6 @@ main (int argc, char *argv[]) */ const char *cmd[] = { "nbdkit", "-s", "--exit-with-parent", "sh", script, NULL }; - int i; progname = argv[0]; @@ -147,15 +146,13 @@ main (int argc, char *argv[]) } /* Attempt to set a bitmask with an unknown bit. */ - i = LIBNBD_HANDSHAKE_FLAG_NO_ZEROES << 1; - if (nbd_set_handshake_flags (nbd, i) != -1) { + if (nbd_set_handshake_flags (nbd, LIBNBD_HANDSHAKE_FLAG_MASK + 1) != -1) { fprintf (stderr, "%s: test failed: " "nbd_set_handshake_flags did not reject invalid bitmask\n", argv[0]); exit (EXIT_FAILURE); } - i = LIBNBD_HANDSHAKE_FLAG_FIXED_NEWSTYLE | LIBNBD_HANDSHAKE_FLAG_NO_ZEROES; - if (nbd_get_handshake_flags (nbd) != i) { + if (nbd_get_handshake_flags (nbd) != LIBNBD_HANDSHAKE_FLAG_MASK) { fprintf (stderr, "%s: test failed: " "nbd_get_handshake_flags not left at default value\n", argv[0]); @@ -247,7 +244,7 @@ main (int argc, char *argv[]) check (EINVAL, "nbd_pread: "); /* Use unknown command flags */ - if (nbd_pread (nbd, buf, 1, 0, -1) != -1) { + if (nbd_pread (nbd, buf, 1, 0, LIBNBD_CMD_FLAG_MASK + 1) != -1) { fprintf (stderr, "%s: test failed: " "nbd_pread did not fail with bogus flags\n", argv[0]); diff --git a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go index 9af0ed8..9e782ac 100644 --- a/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go +++ b/golang/src/libguestfs.org/libnbd/libnbd_110_defaults_test.go @@ -63,7 +63,7 @@ func Test110Defaults(t *testing.T) { if err != nil { t.Fatalf("could not get handshake flags: %s", err) } - if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES { + if flags != HANDSHAKE_FLAG_MASK { t.Fatalf("unexpected handshake flags") } diff --git a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go index 5d8cce7..b0b06bd 100644 --- a/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go +++ b/golang/src/libguestfs.org/libnbd/libnbd_120_set_non_defaults_test.go @@ -93,7 +93,7 @@ func Test120SetNonDefaults(t *testing.T) { t.Fatalf("unexpected structured replies state") } - err = h.SetHandshakeFlags(HANDSHAKE_FLAG_NO_ZEROES << 1) + err = h.SetHandshakeFlags(HANDSHAKE_FLAG_MASK + 1) if err == nil { t.Fatalf("expect failure for out-of-range flags") } @@ -101,7 +101,7 @@ func Test120SetNonDefaults(t *testing.T) { if err != nil { t.Fatalf("could not get handshake flags: %s", err) } - if flags != HANDSHAKE_FLAG_FIXED_NEWSTYLE | HANDSHAKE_FLAG_NO_ZEROES { + if flags != HANDSHAKE_FLAG_MASK { t.Fatalf("unexpected handshake flags") } -- 2.28.0 _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://www.redhat.com/mailman/listinfo/libguestfs