On Mon, Jun 17, 2019 at 08:39:43PM -0500, Eric Blake wrote:
Exploration of what it would take to treat the nbd_pread_callback
status parameter as a language-appropriate enum rather than a bare
integer. While I've identified a rough set of the places to change, I
have not actually got it working with Python or OCaml bindings.
---
Posting this more for an idea of whether we want to pursue it
further. I have no hard feelings if we decide to ditch it; what's
more, compilers don't necessarily guarantee ABI stability with enum
types in public headers, so sticking to Int may be better after all.
I believe there are ABI problems with enums (something about
how they change ABI if there are more than 256 cases?)
generator/generator | 46
+++++++++++++++++++++++++++++++++++++++------
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/generator/generator b/generator/generator
index 630260b..d269a04 100755
--- a/generator/generator
+++ b/generator/generator
@@ -845,6 +845,7 @@ and arg =
| Int64 of string (* 64 bit signed int *)
| Opaque of string (* opaque object, void* in C *)
| Path of string (* filename or path *)
+| ReadStatus of string (* enum of pread_callback status *)
| SockAddrAndLen of string * string (* struct sockaddr * + socklen_t *)
| String of string (* string *)
| StringList of string (* argv-style NULL-terminated array of strings *)
@@ -2033,10 +2034,13 @@ let constants = [
"CMD_FLAG_NO_HOLE", 1 lsl 1;
"CMD_FLAG_DF", 1 lsl 2;
"CMD_FLAG_REQ_ONE", 1 lsl 3;
-
- "READ_DATA", 1;
- "READ_HOLE", 2;
- "READ_ERROR", 3;
+]
+let enums = [
+ "nbd_read_status", [
+ "READ_DATA", 1;
+ "READ_HOLE", 2;
+ "READ_ERROR", 3;
+ ];
]
This is an assoc-list (
https://en.wikipedia.org/wiki/Association_list)
so you can look up entries using:
List.assoc "nbd_read_status" enums
which will return the sub-list if you need to do that. It's not very
time efficient but for the tiny number of cases we expect it's not a
problem.
(*----------------------------------------------------------------------*)
@@ -2812,6 +2816,7 @@ let rec name_of_arg = function
| Int64 n -> [n]
| Opaque n -> [n]
| Path n -> [n]
+| ReadStatus n -> [n]
| SockAddrAndLen (n, len) -> [n; len]
| String n -> [n]
| StringList n -> [n]
@@ -2845,6 +2850,7 @@ let rec print_c_arg_list ?(handle = false) args =
| Int n -> pr "int %s" n
| Int64 n -> pr "int64_t %s" n
| Opaque n -> pr "void *%s" n
+ | ReadStatus n -> pr "enum nbd_read_status %s" n
| Path n
| String n -> pr "const char *%s" n
| StringList n -> pr "char **%s" n
@@ -2896,6 +2902,13 @@ let generate_include_libnbd_h () =
pr "\n";
List.iter (fun (n, i) -> pr "#define LIBNBD_%-30s %d\n" n i) constants;
pr "\n";
+ List.iter (
+ fun (name, list) ->
+ pr "enum %s {\n" name;
+ List.iter (fun (n, i) -> pr " LIBNBD_%-30s = %d,\n" n i) list;
+ pr "};\n"
+ ) enums;
+ pr "\n";
pr "extern struct nbd_handle *nbd_create (void);\n";
pr "#define LIBNBD_HAVE_NBD_CREATE 1\n";
pr "\n";
@@ -3367,6 +3380,7 @@ let print_python_binding name { args; ret } =
| Int _ -> ()
| Opaque n ->
pr " struct %s_%s_data *_data = %s;\n" name cb_name n
+ | ReadStatus n
| String n
| UInt64 n -> ()
(* The following not yet implemented for callbacks XXX *)
@@ -3384,6 +3398,7 @@ let print_python_binding name { args; ret } =
| BytesIn (n, len) -> pr " \"y#\""
| Int n -> pr " \"i\""
| Opaque n -> pr " \"O\""
+ | ReadStatus n -> pr " XXX1"
| String n -> pr " \"s\""
| UInt64 n -> pr " \"K\""
(* The following not yet implemented for callbacks XXX *)
@@ -3398,6 +3413,7 @@ let print_python_binding name { args; ret } =
| ArrayAndLen (UInt32 n, _) -> pr ", py_%s" n
| BytesIn (n, len) -> pr ", %s, (int) %s" n len
| Opaque _ -> pr ", _data->data"
+ | ReadStatus n -> pr ", XXX2 %s" n
| Int n
| String n
| UInt64 n -> pr ", %s" n
@@ -3436,6 +3452,7 @@ let print_python_binding name { args; ret } =
| BytesIn _
| Int _
| Opaque _
+ | ReadStatus _
| String _
| UInt64 _ -> ()
(* The following not yet implemented for callbacks XXX *)
@@ -3497,6 +3514,7 @@ let print_python_binding name { args; ret } =
pr " long long %s; /* really int64_t */\n" n
| Opaque _ -> ()
| Path n -> pr " char *%s = NULL;\n" n
+ | ReadStatus n -> pr " XXX3 %s\n" n
| SockAddrAndLen (n, _) ->
pr " /* XXX Complicated - Python uses a tuple of different\n";
pr " * lengths for the different socket types.\n";
@@ -3537,6 +3555,7 @@ let print_python_binding name { args; ret } =
| Int64 _
| Opaque _
| Path _
+ | ReadStatus _
| SockAddrAndLen _
| String _
| StringList _
@@ -3561,6 +3580,7 @@ let print_python_binding name { args; ret } =
| Int64 n -> pr " \"L\""
| Opaque _ -> pr " \"O\""
| Path n -> pr " \"O&\""
+ | ReadStatus n -> pr " XXX4"
| SockAddrAndLen (n, _) -> pr " \"O\""
| String n -> pr " \"s\""
| StringList n -> pr " \"O\""
@@ -3584,6 +3604,7 @@ let print_python_binding name { args; ret } =
| Int n -> pr ", &%s" n
| Int64 n -> pr ", &%s" n
| Opaque n -> pr ", &%s_data->data" (find_callback n)
+ | ReadStatus n -> pr ", XXX5 &%s" n
| Path n -> pr ", PyUnicode_FSConverter, &%s" n
| SockAddrAndLen (n, _) -> pr ", &%s" n
| String n -> pr ", &%s" n
@@ -3634,6 +3655,7 @@ let print_python_binding name { args; ret } =
| Int64 n -> pr " %s_i64 = %s;\n" n n
| Opaque n -> ()
| Path _ -> ()
+ | ReadStatus n -> pr " XXX6 %s\n" n
| SockAddrAndLen _ ->
pr " abort (); /* XXX SockAddrAndLen not implemented */\n";
| String _ -> ()
@@ -3662,6 +3684,7 @@ let print_python_binding name { args; ret } =
| Int64 n -> pr ", %s_i64" n
| Opaque n -> pr ", %s_data" (find_callback n)
| Path n -> pr ", %s" n
+ | ReadStatus n -> pr ", XXX8 %s" n
| SockAddrAndLen (n, _) -> pr ", /* XXX */ (void *) %s, 0" n
| String n -> pr ", %s" n
| StringList n -> pr ", %s" n
@@ -3696,6 +3719,7 @@ let print_python_binding name { args; ret } =
| Int64 _
| Opaque _
| Path _
+ | ReadStatus _
| SockAddrAndLen _
| String _
| StringList _
@@ -3741,6 +3765,7 @@ let print_python_binding name { args; ret } =
| Int64 _ -> ()
| Opaque _ -> ()
| Path n -> pr " free (%s);\n" n
+ | ReadStatus _ -> ()
| SockAddrAndLen _ -> ()
| String n -> ()
| StringList n -> pr " nbd_internal_py_free_string_list (%s);\n" n
@@ -3795,6 +3820,7 @@ import libnbdmod
";
List.iter (fun (n, i) -> pr "%-30s = %d\n" n i) constants;
+ (* how to represent enums? *)
pr "\
@@ -3839,6 +3865,7 @@ class NBD (object):
| Int64 n -> n, None
| Opaque n -> n, None
| Path n -> n, None
+ | ReadStatus n -> n, None
| SockAddrAndLen (n, _) -> n, None
| String n -> n, None
| StringList n -> n, None
@@ -3929,6 +3956,7 @@ and ocaml_arg_to_string = function
| OCamlArg (Int64 _) -> "int64"
| OCamlArg (Opaque n) -> sprintf "'%s" n
| OCamlArg (Path _) -> "string"
+ | OCamlArg (ReadStatus _) -> "int"
| OCamlArg (SockAddrAndLen _) -> "string" (* XXX not impl *)
| OCamlArg (String _) -> "string"
| OCamlArg (StringList _) -> "string list"
@@ -3978,6 +4006,7 @@ exception Closed of string
List.iter (
fun (n, _) -> pr "val %s : int32\n" (String.lowercase_ascii n)
) constants;
+ (* how to represent enums? *)
pr "\n";
pr "\
@@ -4040,6 +4069,7 @@ let () =
List.iter (
fun (n, i) -> pr "let %s = %d_l\n" (String.lowercase_ascii n) i
) constants;
+ (* how to represent enums? *)
Something like:
type name_of_the_enum =
| CaseA
| CaseB
You have to at least make sure the first letter is a capital (or
capitalize it) unfortunately.
pr "\n";
pr "\
@@ -4092,7 +4122,7 @@ let print_ocaml_binding (name, { args; ret }) =
List.map (
function
| ArrayAndLen (UInt32 n, _) | BytesIn (n, _)
- | Int n | Opaque n | String n | UInt64 n ->
+ | Int n | Opaque n | ReadStatus n | String n | UInt64 n ->
n ^ "v"
(* The following not yet implemented for callbacks XXX *)
| ArrayAndLen _ | Bool _ | BytesOut _
@@ -4124,7 +4154,8 @@ let print_ocaml_binding (name, { args; ret }) =
| BytesIn (n, len) ->
pr " %sv = caml_alloc_string (%s);\n" n len;
pr " memcpy (String_val (%sv), %s, %s);\n" n n len
- | Int n ->
+ | Int n
+ | ReadStatus n ->
pr " %sv = caml_copy_int32 (%s);\n" n n
In OCaml, a union type where there is no parameter (as in
this case) is represented by an OCaml int, so you can write:
%sv = Val_int (%s);
| String n ->
pr " %sv = caml_copy_string (%s);\n" n n
@@ -4282,6 +4313,8 @@ let print_ocaml_binding (name, { args; ret }) =
)
| OCamlArg (Path n) | OCamlArg (String n) ->
pr " const char *%s = String_val (%sv);\n" n n
+ | OCamlArg (ReadStatus n) ->
+ pr " unsigned %s = Int_val (%sv);\n" n n
And I believe this is correct, although I guess you meant to
use enum as the C type?
Rich.
| OCamlArg (SockAddrAndLen (n, len)) ->
pr " const struct sockaddr *%s;\n" n;
pr " socklen_t %s;\n" len;
@@ -4363,6 +4396,7 @@ let print_ocaml_binding (name, { args; ret }) =
| OCamlArg (Int64 _)
| OCamlArg (Opaque _)
| OCamlArg (Path _)
+ | OCamlArg (ReadStatus _)
| OCamlArg (String _)
| OCamlArg (SockAddrAndLen _)
| OCamlArg (UInt _)
--
2.20.1
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top