The return value is a bitmask, and is already documented as never failing, hence it makes sense to return an unsigned value to make it easier to do bitwise operations without worrying about potential undefined C semantics on a signed value. --- I'm not sure if we want this, or even if we do if my OCaml was the most concise, but it matches the sentiment of our recent effort to let the valid_flag and status parameters to callbacks be unsigned. generator/generator | 93 +++++++++++++++++++++++++++++---------------- lib/is-state.c | 2 +- 2 files changed, 61 insertions(+), 34 deletions(-) diff --git a/generator/generator b/generator/generator index 2cd83f1..bb3a1b5 100755 --- a/generator/generator +++ b/generator/generator @@ -873,6 +873,7 @@ and ret = | RInt (* return a small int, -1 = error *) | RInt64 (* 64 bit int, -1 = error *) | RString (* return a newly allocated string, caller frees *) +| RUInt (* return a bitmask, no error possible *) and permitted_state = | Created (* can be called in the START state *) | Connecting (* can be called when connecting/handshaking *) @@ -2013,7 +2014,7 @@ Do not do anything else with the file descriptor."; "aio_get_direction", { default_call with - args = []; ret = RInt; is_locked = false; may_set_error = false; + args = []; ret = RUInt; is_locked = false; may_set_error = false; shortdesc = "return the read or write direction"; longdesc = "\ Return the current direction of this connection, which means @@ -3040,13 +3041,15 @@ let () = (* !may_set_error is incompatible with permitted_states != [] because * an incorrect state will result in set_error being called by the - * generated wrapper. + * generated wrapper. It is also incompatible with RUint. *) List.iter ( function | name, { permitted_states = (_::_); may_set_error = false } -> failwithf "%s: if may_set_error is false, permitted_states must be empty (any permitted state)" name + | name, { ret = RUInt; may_set_error = true } -> + failwithf "%s: if ret is RUInt, may_set_error must be false" name | _ -> () ) handle_calls @@ -3189,6 +3192,7 @@ let print_call name args ret = | RConstString -> pr "const char *" | RInt64 -> pr "int64_t " | RString -> pr "char *" + | RUInt -> pr "unsigned " ); pr "nbd_%s " name; print_arg_list ~handle:true args @@ -3329,10 +3333,11 @@ let generate_lib_api_c () = | RBool | RErr | RFd - | RInt -> "int", "-1" - | RConstString -> "const char *", "NULL" - | RInt64 -> "int64_t", "-1" - | RString -> "char *", "NULL" in + | RInt -> "int", Some "-1" + | RConstString -> "const char *", Some "NULL" + | RInt64 -> "int64_t", Some "-1" + | RString -> "char *", Some "NULL" + | RUInt -> "unsigned", None in pr "%s\n" ret_c_type; pr "nbd_%s " name; @@ -3347,6 +3352,7 @@ let generate_lib_api_c () = | RConstString -> pr " const char *ret;\n" | RInt64 -> pr " int64_t ret;\n" | RString -> pr " char *ret;\n" + | RUInt -> pr " unsigned ret;\n" ); pr "\n"; if may_set_error then ( @@ -3356,8 +3362,11 @@ let generate_lib_api_c () = if is_locked then pr " pthread_mutex_lock (&h->lock);\n"; if permitted_states <> [] then ( + let value = match errcode with + | Some value -> value + | None -> assert false in pr " if (!%s_in_permitted_state (h)) {\n" name; - pr " ret = %s;\n" errcode; + pr " ret = %s;\n" value; pr " goto out;\n"; pr " }\n" ); @@ -3412,31 +3421,37 @@ let print_api (name, { args; ret; permitted_states; shortdesc; longdesc; match ret with | RBool -> pr "This call returns a boolean value.\n"; - "-1" + Some "-1" | RConstString -> pr "This call returns a statically allocated string.\n"; pr "You B<must not> try to free the string.\n"; - "NULL" + Some "NULL" | RErr -> pr "If the call is successful the function returns C<0>.\n"; - "-1" + Some "-1" | RFd -> pr "This call returns a file descriptor.\n"; - "-1" + Some "-1" | RInt -> pr "This call returns an integer E<ge> 0.\n"; - "-1" + Some "-1" | RInt64 -> pr "This call returns a 64 bit signed integer E<ge> 0.\n"; - "-1" + Some "-1" | RString -> pr "This call returns a string. The caller must free the\n"; pr "returned string to avoid a memory leak.\n"; - "NULL" + Some "NULL" + | RUInt -> + pr "This call returns a bitmask.\n"; + None in pr "\n"; if may_set_error then ( - pr "On error C<%s> is returned.\n" errcode; + let value = match errcode with + | Some value -> value + | None -> assert false in + pr "On error C<%s> is returned.\n" value; pr "See L<libnbd(3)/ERROR HANDLING> for how to get further details\n"; pr "of the error.\n" ) @@ -3683,7 +3698,7 @@ PyInit_libnbdmod (void) } " -let print_python_binding name { args; ret } = +let print_python_binding name { args; ret; may_set_error } = (* Functions with a Closure parameter are special because we * have to generate wrapper functions which translate the * callbacks back to Python. @@ -3835,6 +3850,7 @@ let print_python_binding name { args; ret } = | RConstString -> pr " const char *ret;\n" | RInt64 -> pr " int64_t ret;\n" | RString -> pr " char *ret;\n"; + | RUInt -> pr " unsigned ret;\n" ); pr " PyObject *py_ret;\n"; List.iter ( @@ -4018,14 +4034,17 @@ let print_python_binding name { args; ret } = | UInt64 n -> pr ", %s_u64" n ) args; pr ");\n"; - (match ret with - | RBool | RErr | RFd | RInt | RInt64 -> pr " if (ret == -1) {\n"; - | RConstString | RString -> pr " if (ret == NULL) {\n"; + if may_set_error then ( + (match ret with + | RBool | RErr | RFd | RInt | RInt64 -> pr " if (ret == -1) {\n"; + | RConstString | RString -> pr " if (ret == NULL) {\n"; + | RUInt -> assert false + ); + pr " raise_exception ();\n"; + pr " py_ret = NULL;\n"; + pr " goto out;\n"; + pr " }\n" ); - pr " raise_exception ();\n"; - pr " py_ret = NULL;\n"; - pr " goto out;\n"; - pr " }\n"; (* Convert the result back to a Python object and return it. *) let use_ret = ref true in @@ -4062,7 +4081,8 @@ let print_python_binding name { args; ret } = pr " py_ret = Py_None;\n"; pr " Py_INCREF (py_ret);\n" | RFd - | RInt -> + | RInt + | RUInt -> pr " py_ret = PyLong_FromLong (ret);\n" | RInt64 -> pr " py_ret = PyLong_FromLongLong (ret);\n" @@ -4072,7 +4092,9 @@ let print_python_binding name { args; ret } = ); pr "\n"; - pr " out:\n"; + if may_set_error then ( + pr " out:\n" + ); List.iter ( function | ArrayAndLen (UInt32 n, len) -> pr " free (%s);\n" n @@ -4347,6 +4369,7 @@ and ocaml_ret_to_string = function | RInt -> "int" | RInt64 -> "int64" | RString -> "string" + | RUInt -> "int" let rec name_of_ocaml_arg = function | OCamlHandle -> "h" @@ -4747,9 +4770,10 @@ let print_ocaml_binding (name, { args; ret }) = let errcode = match ret with - | RBool | RErr | RFd | RInt | RInt64 -> pr " int r;\n"; "-1" - | RConstString -> pr " const char *r;\n"; "NULL" - | RString -> pr " char *r;\n"; "NULL" in + | RBool | RErr | RFd | RInt | RInt64 -> pr " int r;\n"; Some "-1" + | RConstString -> pr " const char *r;\n"; Some "NULL" + | RString -> pr " char *r;\n"; Some "NULL" + | RUInt -> pr " unsigned r;\n"; None in pr "\n"; pr " caml_enter_blocking_section ();\n"; pr " r = nbd_%s " name; @@ -4757,14 +4781,17 @@ let print_ocaml_binding (name, { args; ret }) = pr ";\n"; pr " caml_leave_blocking_section ();\n"; pr "\n"; - pr " if (r == %s)\n" errcode; - pr " nbd_internal_ocaml_raise_error ();\n"; - pr "\n"; + (match errcode with + | Some code -> + pr " if (r == %s)\n" code; + pr " nbd_internal_ocaml_raise_error ();\n"; + pr "\n" + | None -> () + ); (match ret with | RBool -> pr " rv = Val_bool (r);\n" | RErr -> pr " rv = Val_unit;\n" - | RFd -> pr " rv = Val_int (r);\n" - | RInt -> pr " rv = Val_int (r);\n" + | RFd | RInt | RUInt -> pr " rv = Val_int (r);\n" | RInt64 -> pr " rv = caml_copy_int64 (r);\n" | RConstString -> pr " rv = caml_copy_string (r);\n" | RString -> diff --git a/lib/is-state.c b/lib/is-state.c index d3335fb..1a85c7a 100644 --- a/lib/is-state.c +++ b/lib/is-state.c @@ -144,7 +144,7 @@ nbd_unlocked_aio_is_closed (struct nbd_handle *h) return nbd_internal_is_state_closed (get_public_state (h)); } -int +unsigned nbd_unlocked_aio_get_direction (struct nbd_handle *h) { return nbd_internal_aio_get_direction (get_public_state (h)); -- 2.20.1