Re: [Xen-devel] [PATCH v4 05/15] oxenstored: add support for systemd active sockets

["Luis R. Rodriguez" <mcgrof@xxxxxxxx>]

On Wed, Apr 30, 2014 at 10:27:43AM +0100, Anil Madhavapeddy wrote:
> On 30 Apr 2014, at 02:11, Luis R. Rodriguez <mcgrof@xxxxxxxxxxxxxxxx> wrote:
> 
> > From: "Luis R. Rodriguez" <mcgrof@xxxxxxxx>
> > 
> > This adds systemd socket activation support for the Ocaml xenstored.
> > Ocaml lacks systemd library support so we provide our own C helpers
> > as is done with other functionality lacking on Ocaml.
> > 
> <snip>
> 
> The definition of sd_active_socket_required (unit ->int) below doesn't
> match the C stub:
> 
> > +
> > +external sd_listen_fds: string -> Unix.file_descr = "ocaml_sd_listen_fds"
> > +external sd_active_socket_required: unit -> int = 
> > "ocaml_sd_active_socket_required"
> 
> > +
> > +CAMLprim value ocaml_sd_active_socket_required(void)
> > +{
> > +   CAMLparam0();
> > +   CAMLlocal1(ret);
> > +
> > +   ret = Val_int(0);
> > +
> > +   CAMLreturn(ret);
> > +}
> > +#endif
> 
> That should be:
> 
> CAMLprim value ocaml_sd_active_socket_required(value u)
> {
>   CAMLparam1(u);
>   CAMLlocal1(ret);
>   ret = Val_int(0);
>   CAMLreturn(ret);
> }

Do we have to require an argument though?

> (Since the function isn't doing any heap allocation, it's actually
> just safe to compress it to "return (Val_int(0))", but it's probably
> better to keep it like this to avoid any surprises).
> 
> As Dave notes, returning Val_true and setting the OCaml signature to
> bool would be a clearer interface.

OK thanks for the review! Let me know what you think of the other changes
I sent in reply to Dave.

  Luis

Attachment: pgpDa_LnzEfmN.pgp
Description: PGP signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel