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

[Anil Madhavapeddy <anil@xxxxxxxxxx>]

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>

-anil


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