----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1557/#review1495 -----------------------------------------------------------
I've added some embedded comments. I have at least one more I need to write up. trunk/qpid/cpp/include/qmf/EventNotifier.h <https://reviews.apache.org/r/1557/#comment3428> The session arguments should be passed as references, not pointers. Pointers can be null and you will have to deal with that case whereas references cannot be null and are generally cleaner and more elegant. trunk/qpid/cpp/src/qmf/AgentSessionImpl.h <https://reviews.apache.org/r/1557/#comment3429> Again, please use references rather than pointers. trunk/qpid/cpp/src/qmf/AgentSessionImpl.h <https://reviews.apache.org/r/1557/#comment3430> This set of pointers is going to cause a memory leak when the AgentImpl is destroyed. It would be better to use a boost::shared_ptr instead of a raw pointer. trunk/qpid/cpp/src/qmf/EventNotifierImpl.cpp <https://reviews.apache.org/r/1557/#comment3431> What is this doing here? - Ted On 2011-08-17 14:07:58, Darryl Pierce wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1557/ > ----------------------------------------------------------- > > (Updated 2011-08-17 14:07:58) > > > Review request for qpid, Kenneth Giusti, michael goulish, and Ted Ross. > > > Summary > ------- > > Provides a new method for providing notification to an interested party when > new messages are received. > > The EventNotifier class can be associated with either a console or agent > session. The object provides a file descriptor which then becomes readable > when there are messages to be processed. > > This implementation only supports Posix. There is some work necessary to get > a Windows implementation in place. > > > Diffs > ----- > > trunk/qpid/cpp/bindings/qmf2/examples/cpp/Makefile.am 1158370 > trunk/qpid/cpp/bindings/qmf2/examples/cpp/event_driven_list_agents.cpp > PRE-CREATION > trunk/qpid/cpp/include/qmf/AgentSession.h 1158370 > trunk/qpid/cpp/include/qmf/ConsoleSession.h 1158370 > trunk/qpid/cpp/include/qmf/EventNotifier.h PRE-CREATION > trunk/qpid/cpp/src/CMakeLists.txt 1158370 > trunk/qpid/cpp/src/qmf.mk 1158370 > trunk/qpid/cpp/src/qmf/AgentSession.cpp 1158370 > trunk/qpid/cpp/src/qmf/AgentSessionImpl.h PRE-CREATION > trunk/qpid/cpp/src/qmf/ConsoleSession.cpp 1158370 > trunk/qpid/cpp/src/qmf/ConsoleSessionImpl.h 1158370 > trunk/qpid/cpp/src/qmf/EventNotifier.cpp PRE-CREATION > trunk/qpid/cpp/src/qmf/EventNotifierImpl.h PRE-CREATION > trunk/qpid/cpp/src/qmf/EventNotifierImpl.cpp PRE-CREATION > trunk/qpid/cpp/src/qmf/PosixEventNotifierImpl.cpp PRE-CREATION > trunk/qpid/cpp/src/tests/EventNotifierTest.cpp PRE-CREATION > trunk/qpid/cpp/src/tests/Makefile.am 1158370 > > Diff: https://reviews.apache.org/r/1557/diff > > > Testing > ------- > > An example agent takes the existing list_agents and uses an EventNotifier to > respond to incoming messages rather than blocking on the > ConsoleSession.nextReceiver() API. > > A unit test verifies that the file handle provides by the EventNotifier type > is properly updating on incoming messgaes. > > > Thanks, > > Darryl > >
