[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 30 Apr 2014, at 18:35, Luis R. Rodriguez <mcgrof@xxxxxxxx> wrote:
> Do we have to require an argument though?

External bindings needs to be functions, yes.  The `unit` argument ensures
that you can call the binding multiple times.  It's a little wasteful to
have a C binding just to return a constant compile-time value, and this
could be improved by using optcomp (the OCaml equivalent of cpp) to put
this logic into the OCaml code rather than C.  A separate patchset, 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.

Look good to me with the argument addition and switch to bool;

Acked-by: Anil Madhavapeddy <anil@xxxxxxxxxx>


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.