qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Stitcher" <astitc...@apache.org>
Subject Re: Review Request: Provides the EventNotifier type, plus example agent and unit test.
Date Mon, 22 Aug 2011 14:26:49 GMT


> On 2011-08-19 21:47:12, Kenneth Giusti wrote:
> > trunk/qpid/cpp/src/qmf/EventNotifierImpl.h, line 36
> > <https://reviews.apache.org/r/1557/diff/1/?file=33112#file33112line36>
> >
> >     May I make a suggestion - take a look at qpid::sys::IOHandle in the qpid code.
 This is an abstract class that hides the OS-specific bits (fd/Socket) pretty well.  We could
do something _like_ that here - add another level of abstraction by having getHandle() return
a class instead of an 'int'.
> >     
> >     Or, perhaps not as "pretty" just typedef the return value using different OS-specific
conditional compile:
> >     
> >     #if defined(_WIN32)
> >     #include <whatever windoze headers>
> >     typedef <whatever windoze stuff> IOHandle;
> >     #else
> >     typedef int IOHandle;
> >     #endif
> >     
> >     then we define:
> >     
> >     IOHandle getHandle() const;
> >     
> >     
> >

The trouble is copying IOHandle can't help you!

Ultimately providing an OS specific handle from an OS independent abstraction can't be done
in an OS independent way. You always need to have a particular API with an OS specific signature
to return the OS specific handle so you can use that handle together with your other application
specific handling, at the very least the operation of returning an OS handle from an IOHandle
is a non-portable part of the API.

I suppose if that is the only non-portable part of the API then you're in a better shape than
before, and it may be possible to do something clever(ish) with templates to avoid the actual
header files being different so that might be a direction:

Something like:

template <typename OSHandle>
OSHandle getOSHandle(IOHandle&);

and then define

template int getOSHandle<int>(IOHandle&); for unix and
template HANDLE getOSHandle<HANDLE>(IOHandle&); for Windows.

- Completely untested thoughts - worth practically what you paid for them.


- Andrew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1557/#review1569
-----------------------------------------------------------


On 2011-08-19 18:30:06, Darryl Pierce wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1557/
> -----------------------------------------------------------
> 
> (Updated 2011-08-19 18:30:06)
> 
> 
> 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 1159329 
>   trunk/qpid/cpp/bindings/qmf2/examples/cpp/event_driven_list_agents.cpp PRE-CREATION

>   trunk/qpid/cpp/include/qmf/AgentSession.h 1159329 
>   trunk/qpid/cpp/include/qmf/ConsoleSession.h 1159329 
>   trunk/qpid/cpp/include/qmf/EventNotifier.h PRE-CREATION 
>   trunk/qpid/cpp/src/CMakeLists.txt 1159329 
>   trunk/qpid/cpp/src/qmf.mk 1159329 
>   trunk/qpid/cpp/src/qmf/AgentSession.cpp 1159329 
>   trunk/qpid/cpp/src/qmf/AgentSessionImpl.h PRE-CREATION 
>   trunk/qpid/cpp/src/qmf/ConsoleSession.cpp 1159329 
>   trunk/qpid/cpp/src/qmf/ConsoleSessionImpl.h 1159329 
>   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 1159329 
> 
> 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