On Wed, Nov 17, 2021 at 05:58:53PM +0000, Richard W.M. Jones wrote:
Using the function code_of_unix_error from <caml/unixsupport.h>
we can
greatly simplify this function. code_of_unix_error was added in OCaml
4.01 which is ≤ 4.03 that we currently require.
See also:
https://github.com/ocaml/ocaml/issues/4812
This does require a small change ot how OCaml plugins are linked -- we
now need to link them with the standard OCaml Unix library (unix.cmxa).
This commit also adds a comprehensive end-to-end test of error codes.
---
plugins/cc/nbdkit-cc-plugin.pod | 4 +-
plugins/ocaml/nbdkit-ocaml-plugin.pod | 2 +-
plugins/ocaml/Makefile.am | 2 +-
tests/Makefile.am | 20 +++++-
plugins/ocaml/NBDKit.ml | 25 +------
plugins/ocaml/bindings.c | 22 +------
tests/test-cc-ocaml.sh | 2 +-
tests/cc_shebang.ml | 2 +-
tests/test-ocaml-errorcodes.c | 95 +++++++++++++++++++++++++++
tests/test_ocaml_errorcodes_plugin.ml | 32 +++++++++
.gitignore | 1 +
11 files changed, 155 insertions(+), 52 deletions(-)
diff --git a/plugins/cc/nbdkit-cc-plugin.pod b/plugins/cc/nbdkit-cc-plugin.pod
index 0fe0d9ea..be4019f9 100644
--- a/plugins/cc/nbdkit-cc-plugin.pod
+++ b/plugins/cc/nbdkit-cc-plugin.pod
@@ -89,7 +89,7 @@ C<CC=g++> as a parameter to exec nbdkit.
=head2 Using this plugin with OCaml
nbdkit cc CC=ocamlopt \
- CFLAGS="-output-obj -runtime-variant _pic NBDKit.cmx -cclib
-lnbdkitocaml" \
+ CFLAGS="-output-obj -runtime-variant _pic unix.cmxa NBDKit.cmx -cclib
-lnbdkitocaml" \
Worth adding another \-newline split on what is now a long line?
+++ b/plugins/ocaml/Makefile.am
+check_SCRIPTS += \
+ test-ocaml-plugin.so \
+ test-ocaml-errorcodes-plugin.so
+
test-ocaml-plugin.so: test_ocaml_plugin.cmx ../plugins/ocaml/libnbdkitocaml.la
../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx
Less important that the doc long line, but another place where we may
want to consider line splitting or the use of a convenience Makefile
variable to reduce the line length...
$(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml \
-output-obj -runtime-variant _pic -o $@ \
- NBDKit.cmx $< \
+ unix.cmxa NBDKit.cmx $< \
-cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml
test_ocaml_plugin.cmx: test_ocaml_plugin.ml ../plugins/ocaml/NBDKit.cmi
../plugins/ocaml/NBDKit.cmx
$(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml -c $< -o $@
+test-ocaml-errorcodes-plugin.so: test_ocaml_errorcodes_plugin.cmx
../plugins/ocaml/libnbdkitocaml.la ../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx
...particularly since you are repeating some of the same prerequisites
in multiple targets. Maybe:
TEST_OCAML_OBJS = \
../plugins/ocaml/libnbdkitocaml.la \
../plugins/ocaml/NBDKit.cmi \
../plugins/ocaml/NBDKit.cmx \
$(NULL)
test-ocaml-errorcodes-plugin.so: test_ocaml_errorcodes_plugin.cmx $(TEST_OCAML_OBJS)
+++ b/plugins/ocaml/NBDKit.ml
@@ -220,30 +220,7 @@ let register_plugin plugin =
(* Bindings to nbdkit server functions. *)
-external _set_error : int -> unit = "ocaml_nbdkit_set_error" [@@noalloc]
-
-let set_error unix_error =
- (* There's an awkward triple translation going on here, because
- * OCaml Unix.error codes, errno on the host system, and NBD_*
- * errnos are not all the same integer value. Plus we cannot
- * read the host system errno values from OCaml.
- *)
- let nbd_error =
- match unix_error with
- | Unix.EPERM -> 1
- | Unix.EIO -> 2
- | Unix.ENOMEM -> 3
- | Unix.EINVAL -> 4
- | Unix.ENOSPC -> 5
- | Unix.ESHUTDOWN -> 6
- | Unix.EOVERFLOW -> 7
- | Unix.EOPNOTSUPP -> 8
- | Unix.EROFS -> 9
- | Unix.EFBIG -> 10
- | _ -> 4 (* EINVAL *) in
-
- _set_error nbd_error
-
+external set_error : Unix.error -> unit = "ocaml_nbdkit_set_error"
[@@noalloc]
Yep, that's simpler.
+++ b/tests/test-cc-ocaml.sh
@@ -58,6 +58,6 @@ cleanup_fn rm -f $out
rm -f $out
nbdkit -U - cc $script a=1 b=2 c=3 \
- CC="$OCAMLOPT" CFLAGS="-output-obj -runtime-variant _pic -I
$SRCDIR/../plugins/ocaml NBDKit.cmx -cclib -lnbdkitocaml" \
+ CC="$OCAMLOPT" CFLAGS="-output-obj -runtime-variant _pic -I
$SRCDIR/../plugins/ocaml unix.cmxa NBDKit.cmx -cclib -lnbdkitocaml" \
Here, it's harder to split a long line, as you really do have CC="lots of
text"
Series looks good to me, once you decide how you want to handle long lines.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization:
qemu.org |
libvirt.org