> On 2012-01-05 00:41:30, Steve Huston wrote: > > Nit... there are a few trailing spaces, noted by red in the review diff. > > > > There's a qpid/sys/PipeHandle.h that you may be able to use in the > > SignalPipe class, though there may not be too much benefit to that. > > > > In EventStream::next, does the wait on serializer decrease the timeout? If > > not, the wait may end up longer than the caller requested. > > > > Other than that, it's good!
Steve, thanks very much for your time reviewing this code... several times! I will fix the spaces :-) The timeout (qpid::sys::Duration) is converted to an absolute time (targetTimeout) prior to entering the loop. The targetTimeout does not need further adjusting from within the loop. The pre-existing PipeHandle class exposes read and write calls on the pipe whereas the poller's SignalPipe hides them (in order to minimize them). The remaining overlap between them is very small, so I don't think there is much to be gained by trying to re-jig either one. - Cliff ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3386/#review4192 ----------------------------------------------------------- On 2012-01-04 20:12:57, Cliff Jansen wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3386/ > ----------------------------------------------------------- > > (Updated 2012-01-04 20:12:57) > > > Review request for qpid, Andrew Stitcher, Ted Ross, and Steve Huston. > > > Summary > ------- > > Portable qpid::sys::Poller implementation for Posix systems. > > Modelled as closely as possible to the EpollPoller implementation. > > > This addresses bug QPID-3571. > https://issues.apache.org/jira/browse/QPID-3571 > > > Diffs > ----- > > > /svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/posix/PosixPoller.cpp > PRE-CREATION > > Diff: https://reviews.apache.org/r/3386/diff > > > Testing > ------- > > linux > > solaris (x86, sparc, gcc, native sunstudio compilers) > > > Thanks, > > Cliff > >
