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

[Zheng Li <dev@xxxxxxxx>]

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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel