Re: [PATCH 1/8] oxenstored: add a poll-based select mechanism

[Zheng Li <dev@zheng.li>]

On 16/09/2014 10:01, David Scott wrote:
>> +type event = {
>> +	mutable read: bool;
>> +	mutable write: bool;
>> +	mutable except: bool;
>> +}
>
> If I understand correctly this is a representation of the POLL{IN,OUT,PRI} bitmap in the C stubs — might be worth a short comment (to help me remember in future)

Yes, it also corresponds to the read/write/except fd_set as in the existing Unix.select.

It's currently an internal type and not exposed by the module interface, but I agree a comment would be useful to developers.  I will add the in v2.

>> +	let a = Array.make (Hashtbl.length h) (Unix.stdin, init_event ()) in
>
> Might be worth a comment to say that Unix.stdin is a dummy value, since all entries in the array are overwritten below.
>

Will do in v2.

>> +	if (rc < 0) uerror("select", Nothing);
>
> Perhaps the error string should be “poll” so it would be obvious from the Unix_error which implementation failed?
>

Initially it was implemented as a faithful in-place substitution of Unix.select (i.e. same type signature, same semantics, except the 1024 fd limitation), as such we even kept the exception message exactly the same.

Now think twice, I agree with you it's better to be different, especially if we consider to move to a type signature/implementation more natural to poll in the future version (for some performance advantages).

Cheers,
Zheng

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