Skip to content

Commit e15b1cf

Browse files
domob1812domob1812
committed
Various cleanups in zmqnotificationinterface.
This is a pure refactoring of zmqnotificationinterface to make the code easier to read and maintain. It replaces explicit iterators with C++11 for-each loops where appropriate and uses std::unique_ptr to make memory ownership more explicit.
1 parent 78cb45d commit e15b1cf

File tree

3 files changed

+28
-41
lines changed

3 files changed

+28
-41
lines changed

‎src/zmq/zmqnotificationinterface.cpp‎

Lines changed: 18 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -20,33 +20,26 @@ CZMQNotificationInterface::CZMQNotificationInterface() : pcontext(nullptr)
2020
CZMQNotificationInterface::~CZMQNotificationInterface()
2121
{
2222
Shutdown();
23-
24-
for (std::list<CZMQAbstractNotifier*>::iterator i=notifiers.begin(); i!=notifiers.end(); ++i)
25-
{
26-
delete *i;
27-
}
2823
}
2924

3025
std::list<const CZMQAbstractNotifier*> CZMQNotificationInterface::GetActiveNotifiers() const
3126
{
3227
std::list<const CZMQAbstractNotifier*> result;
33-
for (const auto* n : notifiers) {
34-
result.push_back(n);
28+
for (const auto& n : notifiers) {
29+
result.push_back(n.get());
3530
}
3631
return result;
3732
}
3833

3934
CZMQNotificationInterface* CZMQNotificationInterface::Create()
4035
{
41-
CZMQNotificationInterface* notificationInterface = nullptr;
4236
std::map<std::string, CZMQNotifierFactory> factories;
43-
std::list<CZMQAbstractNotifier*> notifiers;
44-
4537
factories["pubhashblock"] = CZMQAbstractNotifier::Create<CZMQPublishHashBlockNotifier>;
4638
factories["pubhashtx"] = CZMQAbstractNotifier::Create<CZMQPublishHashTransactionNotifier>;
4739
factories["pubrawblock"] = CZMQAbstractNotifier::Create<CZMQPublishRawBlockNotifier>;
4840
factories["pubrawtx"] = CZMQAbstractNotifier::Create<CZMQPublishRawTransactionNotifier>;
4941

42+
std::list<std::unique_ptr<CZMQAbstractNotifier>> notifiers;
5043
for (const auto& entry : factories)
5144
{
5245
std::string arg("-zmq" + entry.first);
@@ -58,23 +51,21 @@ CZMQNotificationInterface* CZMQNotificationInterface::Create()
5851
notifier->SetType(entry.first);
5952
notifier->SetAddress(address);
6053
notifier->SetOutboundMessageHighWaterMark(static_cast<int>(gArgs.GetArg(arg + "hwm", CZMQAbstractNotifier::DEFAULT_ZMQ_SNDHWM)));
61-
notifiers.push_back(notifier);
54+
notifiers.emplace_back(notifier);
6255
}
6356
}
6457

6558
if (!notifiers.empty())
6659
{
67-
notificationInterface = new CZMQNotificationInterface();
68-
notificationInterface->notifiers = notifiers;
60+
std::unique_ptr<CZMQNotificationInterface> notificationInterface(new CZMQNotificationInterface());
61+
notificationInterface->notifiers = std::move(notifiers);
6962

70-
if (!notificationInterface->Initialize())
71-
{
72-
delete notificationInterface;
73-
notificationInterface = nullptr;
63+
if (notificationInterface->Initialize()) {
64+
return notificationInterface.release();
7465
}
7566
}
7667

77-
return notificationInterface;
68+
return nullptr;
7869
}
7970

8071
// Called at startup to conditionally set up ZMQ socket(s)
@@ -95,26 +86,15 @@ bool CZMQNotificationInterface::Initialize()
9586
return false;
9687
}
9788

98-
std::list<CZMQAbstractNotifier*>::iterator i=notifiers.begin();
99-
for (; i!=notifiers.end(); ++i)
100-
{
101-
CZMQAbstractNotifier *notifier = *i;
102-
if (notifier->Initialize(pcontext))
103-
{
89+
for (auto& notifier : notifiers) {
90+
if (notifier->Initialize(pcontext)) {
10491
LogPrint(BCLog::ZMQ, "zmq: Notifier %s ready (address = %s)\n", notifier->GetType(), notifier->GetAddress());
105-
}
106-
else
107-
{
92+
} else {
10893
LogPrint(BCLog::ZMQ, "zmq: Notifier %s failed (address = %s)\n", notifier->GetType(), notifier->GetAddress());
109-
break;
94+
return false;
11095
}
11196
}
11297

113-
if (i!=notifiers.end())
114-
{
115-
return false;
116-
}
117-
11898
return true;
11999
}
120100

@@ -124,9 +104,7 @@ void CZMQNotificationInterface::Shutdown()
124104
LogPrint(BCLog::ZMQ, "zmq: Shutdown notification interface\n");
125105
if (pcontext)
126106
{
127-
for (std::list<CZMQAbstractNotifier*>::iterator i=notifiers.begin(); i!=notifiers.end(); ++i)
128-
{
129-
CZMQAbstractNotifier *notifier = *i;
107+
for (auto& notifier : notifiers) {
130108
LogPrint(BCLog::ZMQ, "zmq: Shutdown notifier %s at %s\n", notifier->GetType(), notifier->GetAddress());
131109
notifier->Shutdown();
132110
}
@@ -141,9 +119,9 @@ void CZMQNotificationInterface::UpdatedBlockTip(const CBlockIndex *pindexNew, co
141119
if (fInitialDownload || pindexNew == pindexFork) // In IBD or blocks were disconnected without any new ones
142120
return;
143121

144-
for (std::list<CZMQAbstractNotifier*>::iterator i = notifiers.begin(); i!=notifiers.end(); )
122+
for (auto i = notifiers.begin(); i!=notifiers.end(); )
145123
{
146-
CZMQAbstractNotifier *notifier = *i;
124+
CZMQAbstractNotifier *notifier = i->get();
147125
if (notifier->NotifyBlock(pindexNew))
148126
{
149127
i++;
@@ -162,9 +140,9 @@ void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef&
162140
// all the same external callback.
163141
const CTransaction& tx = *ptx;
164142

165-
for (std::list<CZMQAbstractNotifier*>::iterator i = notifiers.begin(); i!=notifiers.end(); )
143+
for (auto i = notifiers.begin(); i!=notifiers.end(); )
166144
{
167-
CZMQAbstractNotifier *notifier = *i;
145+
CZMQAbstractNotifier *notifier = i->get();
168146
if (notifier->NotifyTransaction(tx))
169147
{
170148
i++;

‎src/zmq/zmqnotificationinterface.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <validationinterface.h>
99
#include <list>
10+
#include <memory>
1011

1112
class CBlockIndex;
1213
class CZMQAbstractNotifier;
@@ -34,7 +35,7 @@ class CZMQNotificationInterface final : public CValidationInterface
3435
CZMQNotificationInterface();
3536

3637
void *pcontext;
37-
std::list<CZMQAbstractNotifier*> notifiers;
38+
std::list<std::unique_ptr<CZMQAbstractNotifier>> notifiers;
3839
};
3940

4041
extern CZMQNotificationInterface* g_zmq_notification_interface;

‎src/zmq/zmqpublishnotifier.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@
1010
#include <util/system.h>
1111
#include <rpc/server.h>
1212

13+
#include <zmq.h>
14+
15+
#include <cstdarg>
16+
#include <cstddef>
17+
#include <map>
18+
#include <string>
19+
#include <utility>
20+
1321
static std::multimap<std::string, CZMQAbstractPublishNotifier*> mapPublishNotifiers;
1422

1523
static const char *MSG_HASHBLOCK = "hashblock";

0 commit comments

Comments
 (0)