Re: [Libguestfs] [libnbd PATCH v3 07/22] generator: Add struct nbd_extent in prep for 64-bit extents

> diff --git a/ocaml/helpers.c b/ocaml/helpers.c > index 3361a696..09666daf 100644 > --- a/ocaml/helpers.c > +++ b/ocaml/helpers.c > @@ -133,6 +133,26 @@ nbd_internal_ocaml_alloc_i64_from_u32_array (uint32_t *a, size_t len) > CAMLreturn (rv); > } > > +value > +nbd_internal_ocaml_alloc_extent64_array (nbd_extent *a, size_t len) > +{ > + CAMLparam0 (); > + CAMLlocal3 (s, v, rv); > + size_t i; > + > + rv = caml_alloc (len, 0); > + for (i = 0; i < len; ++i) { > + s = caml_alloc (2, 0); > + v = caml_copy_int64 (a[i].length); > + Store_field (s, 0, v); > + v = caml_copy_int64 (a[i].flags); > + Store_field (s, 1, v); > + Store_field (rv, i, s); > + } > + > + CAMLreturn (rv); > +} > + > /* Convert a Unix.sockaddr to a C struct sockaddr. */ > void > nbd_internal_unix_sockaddr_to_sa (value sockaddrv, (19) I'd suggest the following addition: > diff --git a/ocaml/helpers.c b/ocaml/helpers.c > index 09666dafa7d1..db652943141d 100644 > --- a/ocaml/helpers.c > +++ b/ocaml/helpers.c > @@ -25,6 +25,7 @@ > #include <string.h> > #include <sys/socket.h> > #include <assert.h> > +#include <inttypes.h> > > #include <caml/alloc.h> > #include <caml/callback.h> > @@ -140,6 +141,16 @@ nbd_internal_ocaml_alloc_extent64_array (nbd_extent *a, size_t len) > CAMLlocal3 (s, v, rv); > size_t i; > > + for (i = 0; i < len; ++i) > + if (a[i].length > INT64_MAX || a[i].flags > INT64_MAX) { > + char errmsg[256]; > + > + snprintf (errmsg, sizeof errmsg, > + "%s: extent[%zu] = { .length = %"PRIu64", .flags = %"PRIu64"}", > + __func__, i, a[i].length, a[i].flags); > + caml_failwith (errmsg); > + } > + > rv = caml_alloc (len, 0); > for (i = 0; i < len; ++i) { > s = caml_alloc (2, 0); *However*, considering the nbd_internal_ocaml_alloc_extent64_array() call site, in the generated extent64_wrapper_locked() function [ocaml/nbd-c.c]: > /* Wrapper for extent64 callback. */ > static int > extent64_wrapper_locked (void *user_data, const char *metacontext, > uint64_t offset, nbd_extent *entries, > size_t nr_entries, int *error) > { > CAMLparam0 (); > CAMLlocal4 (metacontextv, offsetv, entriesv, errorv); > CAMLlocal2 (exn, rv); > const struct user_data *data = user_data; > int r; > value args[4]; > > metacontextv = caml_copy_string (metacontext); > offsetv = caml_copy_int64 (offset); > entriesv = nbd_internal_ocaml_alloc_extent64_array ( > entries, > nr_entries > ); > errorv = caml_alloc_tuple (1); > Store_field (errorv, 0, Val_int (*error)); > args[0] = metacontextv; > args[1] = offsetv; > args[2] = entriesv; > args[3] = errorv; > rv = caml_callbackN_exn (data->fnv, 4, args); > *error = Int_val (Field (errorv, 0)); > if (Is_exception_result (rv)) { > nbd_internal_ocaml_exception_in_wrapper ("extent64", rv); > CAMLreturnT (int, -1); > } > > r = Int_val (rv); > assert (r >= 0); > CAMLreturnT (int, r); > } I'm not sure if raising an OCaml exception like this, in an *inner* C function, is appropriate. caml_failwith() may only be suitable for C functions *directly* called by the OCaml runtime.