Compiling the ActiveMQ-CPP distribution generates hundreds of warnings
on using std::auto_ptr, which has been deprecated since C++11 nearly a
decade ago, and in fact has been removed altogether in C++17.
Is there a show-stopping issue with replacing std::auto_ptr with
std::unique_ptr? I couldn't find any discussion of this. My resources
are limited to the Linux platform (specifically Ubuntu), where I find
that there are very few pain points in the replacement.
For version 3.9.5, using g++ v.6.5.0, compiling the library ('make')
requires only 3 fixups, and compiling the tests ('make check')
requires 3 more.
Of these 6, 5 are the same issue: initializing a std::auto_ptr with
NULL, which is actually unnecessary, and fails for std::unique_ptr
(because NULL causes an overload ambiguity). The fix is simply to
remove the explicit NULL initializer. This is good style anyway, even
with std::auto_ptr retained.
The remaining case is a bit more serious. It arises in
src/test/decaf/net/ssl/SSLSocketFactoryTest.cpp:
std::auto_ptr<Socket> sock( factory->createSocket() );
CPPUNIT_ASSERT( sock != NULL );
The replacement with std::unique_ptr doesn't work because the Socket
class is of an incomplete type here, which std::unique_ptr guards
against (but std::auto_ptr does not). This is actually a buggy test,
with no good way to fix. The following will be "safe", but compilers
will still issue a warning:
Socket* sock( factory->createSocket() );
CPPUNIT_ASSERT( sock != nullptr );
delete sock;
Mostly, this is a deeper underlying issue with the code-base, which
has the distinct feel of "writing Java in C++". Among the prominent
Javaisms is the complete disregard for the fact that C++ supports
covariant return types (probably because Java didn't, until recently).
There was no need for the Socket* pointer in the above test to be of
an incomplete super-type rather than the obvious concrete sub-type.
This is a flaw in the interfaces.
Another significant Javaism is the pervasive use of std::auto_ptr to
begin with. This is a consequence of heap allocation, which is the
only option in Java. But new-ing and delete-ing willy-nilly all over
the place (and recruiting smart pointers to help with the troubles of
doing so) is actually very bad C++, where stack allocation is strongly
preferred. But that's a battle for another day.
I've attached two diff files suitable for the patch program.
libfix.diff - the 3 changes for compiling the library.
checkfix.diff - the 3 changes for compiling the tests.
These can be applied whether or not there is a global search and
replace of std::auto_ptr with std::unique_ptr, which can be done with
a one-liner:
$> cd src
$> perl -i -pe 's/\bauto_ptr\b/unique_ptr/g' $(grep -rl auto_ptr)
With these changes, the library compiles and all tests pass. I don't
have the resources to verify this with other compilers or on other
platforms, which is why I haven't yet investigated how to submit a
pull request.
--
:ar
diff --git a/examples/cmstemplate-stress/Receiver.cpp
b/examples/cmstemplate-stress/Receiver.cpp
index 0b2d35d..23143ec 100755
--- a/examples/cmstemplate-stress/Receiver.cpp
+++ b/examples/cmstemplate-stress/Receiver.cpp
@@ -45,8 +45,8 @@ Receiver::Receiver(const string & url, const string &
queueOrTopicName,
closing(false),
ready(1),
messageListener(NULL),
- cmsTemplate(NULL),
- asyncReceiverThread(NULL),
+ cmsTemplate(),
+ asyncReceiverThread(),
receiveTimeout(receiveTimeout),
bUseThreadPool(useThreadPool),
cmsTemplateCreateTime(0),
diff --git a/examples/cmstemplate-stress/Sender.cpp
b/examples/cmstemplate-stress/Sender.cpp
index ae22129..45cb974 100755
--- a/examples/cmstemplate-stress/Sender.cpp
+++ b/examples/cmstemplate-stress/Sender.cpp
@@ -31,7 +31,7 @@ using namespace cmstemplate;
////////////////////////////////////////////////////////////////////////////////
Sender::Sender(const string& url, const string& queueOrTopicName, bool
isTopic, bool isDeliveryPersistent, int timeToLive) :
- cmsTemplateMutex(), cmsTemplate(NULL) {
+ cmsTemplateMutex(), cmsTemplate() {
ConnectionFactory* connectionFactory =
ConnectionFactoryMgr::getConnectionFactory(url);
diff --git a/main/activemq/core/ActiveMQConnection.cpp
b/main/activemq/core/ActiveMQConnection.cpp
index 15f9cfa..59fd261 100755
--- a/main/activemq/core/ActiveMQConnection.cpp
+++ b/main/activemq/core/ActiveMQConnection.cpp
@@ -265,8 +265,8 @@ namespace core {
optimizedAckScheduledAckInterval(0),
consumerFailoverRedeliveryWaitPeriod(0),
consumerExpiryCheckEnabled(true),
- defaultPrefetchPolicy(NULL),
- defaultRedeliveryPolicy(NULL),
+ defaultPrefetchPolicy(),
+ defaultRedeliveryPolicy(),
exceptionListener(NULL),
transformer(NULL),
connectionInfo(),
diff --git a/test/activemq/core/ActiveMQConnectionTest.cpp
b/test/activemq/core/ActiveMQConnectionTest.cpp
index 5ece63f..ba5947d 100755
--- a/test/activemq/core/ActiveMQConnectionTest.cpp
+++ b/test/activemq/core/ActiveMQConnectionTest.cpp
@@ -171,7 +171,7 @@ namespace {
public:
- TestCloseCancelsHungStartRunnable() : connection(NULL) {
+ TestCloseCancelsHungStartRunnable() : connection() {
}
virtual ~TestCloseCancelsHungStartRunnable() {
diff --git a/test/decaf/net/ServerSocketTest.cpp
b/test/decaf/net/ServerSocketTest.cpp
index 02d936c..dd6a002 100755
--- a/test/decaf/net/ServerSocketTest.cpp
+++ b/test/decaf/net/ServerSocketTest.cpp
@@ -37,7 +37,7 @@ namespace {
std::unique_ptr<Socket> clientS;
int port;
- SocketClient(int port) : Runnable(), clientS(NULL), port(port) {
+ SocketClient(int port) : Runnable(), clientS(), port(port) {
}
virtual void run() {
diff --git a/test/decaf/net/ssl/SSLSocketFactoryTest.cpp
b/test/decaf/net/ssl/SSLSocketFactoryTest.cpp
index 33fe2cc..f64a5dd 100755
--- a/test/decaf/net/ssl/SSLSocketFactoryTest.cpp
+++ b/test/decaf/net/ssl/SSLSocketFactoryTest.cpp
@@ -42,8 +42,9 @@ void SSLSocketFactoryTest::testGetDefault() {
#ifdef HAVE_OPENSSL
- std::unique_ptr<Socket> sock( factory->createSocket() );
- CPPUNIT_ASSERT( sock.get() != NULL );
+ Socket* sock( factory->createSocket() );
+ CPPUNIT_ASSERT( sock != NULL );
+ delete sock;
#else
