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

Reply via email to