On Wednesday, 19 July 2017 22:25:41 CEST Richard W.M. Jones wrote: > On Wed, Jul 19, 2017 at 03:13:47PM +0200, Pino Toscano wrote: > > On Friday, 14 July 2017 15:39:10 CEST Richard W.M. Jones wrote: > > > .gitignore | 6 +- > > > Makefile.am | 2 +- > > > common/mlutils/Makefile.am | 4 - > > > daemon/Makefile.am | 103 +++++++++++++++++++++++-- > > > daemon/chroot.ml | 85 +++++++++++++++++++++ > > > daemon/chroot.mli | 35 +++++++++ > > > daemon/daemon-c.c | 35 +++++++++ > > > daemon/daemon.ml | 39 ++++++++++ > > > daemon/guestfsd.c | 50 ++++++++++++ > > > daemon/sysroot-c.c | 37 +++++++++ > > > daemon/sysroot.ml | 19 +++++ > > > daemon/sysroot.mli | 22 ++++++ > > > daemon/utils.ml | 156 +++++++++++++++++++++++++++++++++++++ > > > daemon/utils.mli | 65 ++++++++++++++++ > > > > TBH I'd just have a single "Daemon" module for the OCaml helpers for > > the daemon, instead of different modules, wirh a single -c.c file for > > all the C implementations. The Sysroot submodule could be implemented > > like the various submodules in Unix_utils. > > Do you mean Daemon.Chroot, Daemon.Sysroot etc?
Yes, exactly.
> > > +val f : t -> ('a -> 'b) -> 'a -> 'b
> > > +(** Run a function in the chroot, returning the result or re-raising
> > > + any exception thrown. *)
> >
> > After reading patch #11, IMHO there should be a variant that takes a
> > generic (unit -> unit) function (called 'fn', maybe?), and have 'f'
> > use it:
> >
> > let f t fun arg =
> > f (fun () -> fun arg)
>
> I'm a bit confused, do you have a compilable example?
Not really without rewriting all of it, but I can improve the snippets.
An interface like:
val f : t -> ('a -> 'b) -> 'a -> 'b
val fn : t -> (unit -> unit) -> 'a
With the implementation like:
let f t func arg =
fn (fun () -> func arg)
let fn func =
...
let ret =
try Either (func ())
with exn -> Or exn in
This way, when calling more more than a single-parameter function, it
is slightly easier to read (IMHO, of course):
let res = Chroot.f chroot (fun () -> ...) in
than
let res = Chroot.f chroot (fun () -> ...) () in
This is mostly syntactic sugar.
> > > +/* Convert an OCaml exception to a reply_with_error_errno call
> > > + * as best we can.
> > > + */
> > > +extern void ocaml_exn_to_reply_with_error (const char *func, value exn);
> > > +
> > > +void
> > > +ocaml_exn_to_reply_with_error (const char *func, value exn)
> > > +{
> >
> > Shouldn't this use CAMLparam1 + CAMLreturn?
>
> It doesn't allocate on the OCaml heap so it should be safe.
>
> > > +let udev_settle ?filename () =
> >
> > Ditto.
>
> ‘Ditto’ means bind the C udev_settle_* functions?
Yes, that's correct.
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
