> On Sept. 19, 2012, 8:42 p.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/QpidDllMain.h,
> >  line 48
> > <https://reviews.apache.org/r/7179/diff/1/?file=158587#file158587line48>
> >
> >     Shouldn't this be defined in the .cpp file?

If the functionality is needed in future in qpidmessaging or qpidbroker, then 
the code can be pulled in to an analogous MessagingDllMain.cpp or 
BrokerDllMain.cpp without duplicating the code (which is already sadly 
duplicated in Thread.cpp).


> On Sept. 19, 2012, 8:42 p.m., Andrew Stitcher wrote:
> > http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/QpidDllMain.h,
> >  line 59
> > <https://reviews.apache.org/r/7179/diff/1/?file=158587#file158587line59>
> >
> >     Currently all exits return TRUE so this line is superfluous.

True, but that is the sole dangerous code path of the four.  My paranoia tells 
me to leave it as is so that someone has to take an extra step to fall through 
to some shared code in the future (and add some descriptive warning comment 
just before the return.

Granted, people regularly put all sorts of code in their DllMain routines.  It 
can be done, but its easy to get wrong.


- Cliff


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


On Sept. 19, 2012, 8:32 p.m., Cliff Jansen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7179/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2012, 8:32 p.m.)
> 
> 
> Review request for qpid, Andrew Stitcher, Chug Rolke, and Steve Huston.
> 
> 
> Description
> -------
> 
> This patch introduces a primitive 
> qpid::sys::SystemInfo::threadSafeShutdown().  It always returns true on Linux 
> and Solaris.  On Windows, it returns true for FreeLibrary and false if exit() 
> has been called, main() returns, or the Qpid libraries have been statically 
> linked.
> 
> It is used to fix the noted static destructor problems in the main Jira.
> 
> Most of the work is plumbing related to obtain knowledge of how the module is 
> terminated in time to be useful to the consumer of the threadSafeShutdown 
> call.
> 
> 
> This addresses bug qpid-4330.
>     https://issues.apache.org/jira/browse/qpid-4330
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/include/qpid/sys/SystemInfo.h
>  1387463 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/CMakeLists.txt 
> 1387463 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/Makefile.am 1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/client/ConnectionImpl.cpp
>  1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/client/windows/ClientDllMain.cpp
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/posix/SystemInfo.cpp
>  1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/solaris/SystemInfo.cpp
>  1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/QpidDllMain.h
>  PRE-CREATION 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/Socket.cpp
>  1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/SystemInfo.cpp
>  1387463 
>   
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/cpp/src/qpid/sys/windows/Thread.cpp
>  1387463 
> 
> Diff: https://reviews.apache.org/r/7179/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Cliff Jansen
> 
>

Reply via email to