[Libguestfs] [libnbd RFC PATCH 9/8] wip: generator: Add ReadStatus enum type
Richard W.M. Jones
rjones at redhat.com
Thu Jun 20 23:42:56 UTC 2019
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 at 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
More information about the Libguestfs
mailing list