qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ted Ross" <tr...@apache.org>
Subject Re: Review Request: Provides the EventNotifier type, plus example agent and unit test.
Date Wed, 17 Aug 2011 14:28:37 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message