Modernise code and use smart pointers and favor std containers over homegrown specialized classes.

7 files changed, 80 insertions, 99 deletions

diff --git a/slobrok/src/vespa/slobrok/server/cmd.cpp b/slobrok/src/vespa/slobrok/server/cmd.cpp
index 6ba4f9a6e98..63a59eb0a8d 100644
--- a/slobrok/src/vespa/slobrok/server/cmd.cpp
+++ b/slobrok/src/vespa/slobrok/server/cmd.cpp
@@ -45,10 +45,9 @@ RegRpcSrvCommand::makeRegRpcSrvCmd(SBEnv &env,
 }
 
 RegRpcSrvCommand
-RegRpcSrvCommand::makeRemRemCmd(SBEnv &env,
-                                const char *name, const char *spec)
+RegRpcSrvCommand::makeRemRemCmd(SBEnv &env, const char *name, const char *spec)
 {
-    RegRpcSrvData *data = new RegRpcSrvData(env, name, spec, NULL);
+    RegRpcSrvData *data = new RegRpcSrvData(env, name, spec, nullptr);
     data->_state = RegRpcSrvData::XCH_IGNORE;
     return RegRpcSrvCommand(data);
 }
@@ -65,8 +64,8 @@ void
 RegRpcSrvCommand::cleanupReservation()
 {
     RpcServerMap &map = _data->env._rpcsrvmap;
-    ReservedName *rsvp = map.getReservation(_data->name.c_str());
-    if (rsvp != NULL && rsvp->isLocal) {
+    const ReservedName *rsvp = map.getReservation(_data->name.c_str());
+    if (rsvp != nullptr && rsvp->isLocal) {
         map.removeReservation(_data->name.c_str());
     }
 }
@@ -74,14 +73,14 @@ RegRpcSrvCommand::cleanupReservation()
 void
 RegRpcSrvCommand::doneHandler(OkState result)
 {
-    LOG_ASSERT(_data != NULL);
+    LOG_ASSERT(_data != nullptr);
 
     if (result.failed()) {
         LOG(warning, "failed in state %d: %s",
             _data->_state, result.errorMsg.c_str());
         cleanupReservation();
         // XXX should handle different state errors differently?
-        if (_data->registerRequest != NULL) {
+        if (_data->registerRequest != nullptr) {
             _data->registerRequest->SetError(FRTE_RPC_METHOD_FAILED,
                                              result.errorMsg.c_str());
             _data->registerRequest->Return();
@@ -121,7 +120,7 @@ RegRpcSrvCommand::doneHandler(OkState result)
  alldone:
     cleanupReservation();
     delete _data;
-    _data = NULL;
+    _data = nullptr;
 }
 
 //-----------------------------------------------------------------------------
diff --git a/slobrok/src/vespa/slobrok/server/reserved_name.h b/slobrok/src/vespa/slobrok/server/reserved_name.h
index 7761dc37918..ee533626f72 100644
--- a/slobrok/src/vespa/slobrok/server/reserved_name.h
+++ b/slobrok/src/vespa/slobrok/server/reserved_name.h
@@ -27,10 +27,10 @@ public:
     {
         _reservedTime.SetNow();
     }
-    bool stillReserved() {
+    bool stillReserved() const {
         return (_reservedTime.MilliSecsToNow() < 15000);
     }
-    int seconds() { return _reservedTime.MilliSecsToNow() / 1000; }
+    int seconds() const { return _reservedTime.MilliSecsToNow() / 1000; }
 };
 
 //-----------------------------------------------------------------------------
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
index 293975cc82a..97749de645c 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_manager.cpp
@@ -142,7 +142,7 @@ RpcServerManager::addMyReservation(const char *name, const char *spec)
 
     // check if we already are in the progress of adding this
     if (_rpcsrvmap.conflictingReservation(name, spec)) {
-        ReservedName * rsv = _rpcsrvmap.getReservation(name);
+        const ReservedName * rsv = _rpcsrvmap.getReservation(name);
         LOG(warning, "conflicting registrations: wanted [%s -> %s] but [%s -> %s] already reserved",
             name, spec, rsv->getName(), rsv->getSpec());
         return OkState(FRTE_RPC_METHOD_FAILED,
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
index 06c5698ec09..47fb7b2516e 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.cpp
@@ -12,33 +12,34 @@ namespace slobrok {
 
 //-----------------------------------------------------------------------------
 
+ManagedRpcServer *
+RpcServerMap::lookupManaged(const char *name) const {
+    auto found = _myrpcsrv_map.find(name);
+    return (found == _myrpcsrv_map.end()) ? nullptr : found->second.get();
+}
+
 NamedService *
 RpcServerMap::lookup(const char *name) const
 {
-    NamedService *d = _myrpcsrv_map[name];
-    return d;
+    return lookupManaged(name);
 }
 
-
 NamedService *
 RpcServerMap::remove(const char *name)
 {
     _visible_map.remove(name);
-    NamedService *d = _myrpcsrv_map.remove(name);
-    return d;
+    auto service = std::move(_myrpcsrv_map[name]);
+    _myrpcsrv_map.erase(name);
+    return service.release();
 }
 
-
-
 std::vector<const NamedService *>
 RpcServerMap::lookupPattern(const char *pattern) const
 {
     std::vector<const NamedService *> retval;
-    for (HashMap<ManagedRpcServer *>::Iterator it = _myrpcsrv_map.iterator();
-         it.valid(); it.next())
-    {
-        if (match(it.key(), pattern)) {
-            retval.push_back(it.value());
+    for (const auto & entry : _myrpcsrv_map) {
+        if (match(entry.first.c_str(), pattern)) {
+            retval.push_back(entry.second.get());
         }
     }
     return retval;
@@ -50,10 +51,8 @@ RpcServerMap::allManaged() const
 {
     std::vector<const NamedService *> retval;
     // get list of all names in myrpcsrv_map
-    for (HashMap<ManagedRpcServer *>::Iterator it = _myrpcsrv_map.iterator();
-         it.valid(); it.next())
-    {
-        retval.push_back(it.value());
+    for (const auto & entry : _myrpcsrv_map) {
+        retval.push_back(entry.second.get());
     }
     return retval;
 }
@@ -64,12 +63,12 @@ RpcServerMap::add(NamedService *rpcsrv)
 {
     const char *name = rpcsrv->getName();
 
-    LOG_ASSERT(rpcsrv != NULL);
-    LOG_ASSERT(_myrpcsrv_map.isSet(name) == false);
+    LOG_ASSERT(rpcsrv != nullptr);
+    LOG_ASSERT(_myrpcsrv_map.find(name) == _myrpcsrv_map.end());
 
     removeReservation(name);
 
-    LOG_ASSERT(_visible_map.lookup(name) == NULL);
+    LOG_ASSERT(_visible_map.lookup(name) == nullptr);
     _visible_map.addNew(rpcsrv);
 }
 
@@ -78,11 +77,12 @@ RpcServerMap::addNew(ManagedRpcServer *rpcsrv)
 {
     const char *name = rpcsrv->getName();
 
-    ManagedRpcServer  *oldman = _myrpcsrv_map.remove(name);
+    auto oldman = std::move(_myrpcsrv_map[name]);
+    _myrpcsrv_map.erase(name);
 
-    if (oldman != NULL) {
-        ReservedName *oldres = _reservations[name];
-        NamedService *oldvis = _visible_map.remove(name);
+    if (oldman) {
+        const ReservedName *oldres = _reservations[name].get();
+        const NamedService *oldvis = _visible_map.remove(name);
 
         const char *spec = rpcsrv->getSpec();
         const char *oldname = oldman->getName();
@@ -90,38 +90,37 @@ RpcServerMap::addNew(ManagedRpcServer *rpcsrv)
         if (strcmp(spec, oldspec) != 0)  {
             LOG(warning, "internal state problem: adding [%s at %s] but already had [%s at %s]",
                 name, spec, oldname, oldspec);
-            if (oldvis != oldman) {
+            if (oldvis != oldman.get()) {
                 const char *n = oldvis->getName();
                 const char *s = oldvis->getSpec();
                 LOG(warning, "BAD: different old visible: [%s at %s]", n, s);
             }
-            if (oldres != NULL) {
+            if (oldres != nullptr) {
                 const char *n = oldres->getName();
                 const char *s = oldres->getSpec();
                 LOG(warning, "old reservation: [%s at %s]", n, s);
             }
         }
-        delete oldman;
     }
     add(rpcsrv);
-    _myrpcsrv_map.set(name, rpcsrv);
+    _myrpcsrv_map[name].reset(rpcsrv);
 }
 
 
 void
 RpcServerMap::addReservation(ReservedName *rpcsrv)
 {
-    LOG_ASSERT(rpcsrv != NULL);
-    LOG_ASSERT(_myrpcsrv_map.isSet(rpcsrv->getName()) == false);
+    LOG_ASSERT(rpcsrv != nullptr);
+    LOG_ASSERT(_myrpcsrv_map.find(rpcsrv->getName()) == _myrpcsrv_map.end());
 
     // must not be reserved for something else already
     // this should have been checked already, so assert
     LOG_ASSERT(! conflictingReservation(rpcsrv->getName(), rpcsrv->getSpec()));
-    ReservedName *old = _reservations.set(rpcsrv->getName(), rpcsrv);
-    LOG_ASSERT(old == NULL
+    auto old = std::move(_reservations[rpcsrv->getName()]);
+    _reservations[rpcsrv->getName()].reset(rpcsrv);
+    LOG_ASSERT(!old
                || strcmp(old->getSpec(), rpcsrv->getSpec()) == 0
                || ! old->stillReserved());
-    delete old;
 }
 
 
@@ -129,37 +128,31 @@ RpcServerMap::addReservation(ReservedName *rpcsrv)
 bool
 RpcServerMap::conflictingReservation(const char *name, const char *spec)
 {
-    ReservedName *resv = _reservations[name];
-    return (resv != NULL &&
+    const ReservedName *resv = _reservations[name].get();
+    return (resv != nullptr &&
             resv->stillReserved() &&
             strcmp(resv->getSpec(), spec) != 0);
 }
 
+const ReservedName *
+RpcServerMap::getReservation(const char *name) const {
+    auto found = _reservations.find(name);
+    return (found == _reservations.end()) ? nullptr : found->second.get();
+}
 
-RpcServerMap::~RpcServerMap()
+RpcServerMap::RpcServerMap()
+    : _myrpcsrv_map(),
+      _reservations()
 {
-    // get list of names in rpcsrv_map
-    std::vector<const char *> names;
-    for (HashMap<ManagedRpcServer *>::Iterator it = _myrpcsrv_map.iterator();
-         it.valid(); it.next())
-    {
-        names.push_back(it.key());
-    }
-
-    for (uint32_t i = 0; i < names.size(); i++) {
-        NamedService *rpcsrv = _myrpcsrv_map.remove(names[i]);
-        LOG_ASSERT(rpcsrv != NULL);
-        delete rpcsrv;
-    }
-    LOG_ASSERT(_myrpcsrv_map.size() == 0);
 }
 
+RpcServerMap::~RpcServerMap() = default;
 
 bool
 RpcServerMap::match(const char *name, const char *pattern)
 {
-    LOG_ASSERT(name != NULL);
-    LOG_ASSERT(pattern != NULL);
+    LOG_ASSERT(name != nullptr);
+    LOG_ASSERT(pattern != nullptr);
     while (*pattern != '\0') {
         if (*name == *pattern) {
             ++name;
@@ -179,8 +172,7 @@ RpcServerMap::match(const char *name, const char *pattern)
 void
 RpcServerMap::removeReservation(const char *name)
 {
-    delete _reservations.remove(name);
+    _reservations.erase(name);
 }
 
-
 } // namespace slobrok
diff --git a/slobrok/src/vespa/slobrok/server/rpc_server_map.h b/slobrok/src/vespa/slobrok/server/rpc_server_map.h
index f25b9989378..4ebccd12b36 100644
--- a/slobrok/src/vespa/slobrok/server/rpc_server_map.h
+++ b/slobrok/src/vespa/slobrok/server/rpc_server_map.h
@@ -2,12 +2,14 @@
 #pragma once
 
 #include "visible_map.h"
+#include <unordered_map>
+#include <string>
+#include <memory>
 
 namespace slobrok {
 
 class NamedService;
 class ManagedRpcServer;
-class RemoteRpcServer;
 class ReservedName;
 
 /**
@@ -19,22 +21,17 @@ class ReservedName;
  * three seperate hashmaps.
  **/
 
-using vespalib::HashMap;
-
 class RpcServerMap
 {
 private:
-    VisibleMap                   _visible_map;
-
-    HashMap<ManagedRpcServer *>  _myrpcsrv_map;
-
-    HashMap<ReservedName *> _reservations;
+    using ManagedRpcServerMap = std::unordered_map<std::string, std::unique_ptr<ManagedRpcServer>>;
+    using ReservedNameMap = std::unordered_map<std::string, std::unique_ptr<ReservedName>>;
+    VisibleMap           _visible_map;
+    ManagedRpcServerMap  _myrpcsrv_map;
+    ReservedNameMap      _reservations;
 
     static bool match(const char *name, const char *pattern);
 
-    RpcServerMap(const RpcServerMap &);            // Not used
-    RpcServerMap &operator=(const RpcServerMap &); // Not use
-
     void add(NamedService *rpcsrv);
 
 public:
@@ -42,9 +39,7 @@ public:
 
     VisibleMap& visibleMap() { return _visible_map; }
 
-    ManagedRpcServer *lookupManaged(const char *name) const {
-        return _myrpcsrv_map[name];
-    }
+    ManagedRpcServer *lookupManaged(const char *name) const;
 
     NamedService *    lookup(const char *name) const;
     RpcSrvlist        lookupPattern(const char *pattern) const;
@@ -57,16 +52,12 @@ public:
     void          addReservation(ReservedName *rpcsrv);
     bool          conflictingReservation(const char *name, const char *spec);
 
-    ReservedName *getReservation(const char *name) const {
-        return _reservations[name];
-    }
+    const ReservedName *getReservation(const char *name) const;
     void removeReservation(const char *name);
 
-    RpcServerMap()
-        : _myrpcsrv_map(NULL),
-          _reservations(NULL)
-    {
-    }
+    RpcServerMap(const RpcServerMap &) = delete;
+    RpcServerMap &operator=(const RpcServerMap &) = delete;
+    RpcServerMap();
     ~RpcServerMap();
 };
 
diff --git a/slobrok/src/vespa/slobrok/server/visible_map.cpp b/slobrok/src/vespa/slobrok/server/visible_map.cpp
index d4487fd0884..4188d1603e3 100644
--- a/slobrok/src/vespa/slobrok/server/visible_map.cpp
+++ b/slobrok/src/vespa/slobrok/server/visible_map.cpp
@@ -87,7 +87,7 @@ VisibleMap::allVisible() const
 void
 VisibleMap::addNew(NamedService *rpcsrv)
 {
-    LOG_ASSERT(rpcsrv != NULL);
+    LOG_ASSERT(rpcsrv != nullptr);
     LOG_ASSERT(_map.isSet(rpcsrv->getName()) == false);
     _map.set(rpcsrv->getName(), rpcsrv);
 
@@ -100,7 +100,7 @@ NamedService *
 VisibleMap::remove(const char *name) {
 
     NamedService *d = _map.remove(name);
-    if (d != NULL) {
+    if (d != nullptr) {
         _history.add(name, _genCnt);
         updated();
     }
@@ -110,10 +110,10 @@ VisibleMap::remove(const char *name) {
 
 NamedService *
 VisibleMap::update(NamedService *rpcsrv) {
-    LOG_ASSERT(rpcsrv != NULL);
+    LOG_ASSERT(rpcsrv != nullptr);
 
     NamedService *d = _map.remove(rpcsrv->getName());
-    LOG_ASSERT(d != NULL);
+    LOG_ASSERT(d != nullptr);
 
     _map.set(rpcsrv->getName(), rpcsrv);
 
@@ -133,7 +133,7 @@ VisibleMap::history(const vespalib::GenCnt& gen) const
          ++it)
     {
         const NamedService *val = lookup(it->c_str());
-        if (val == NULL) {
+        if (val == nullptr) {
             retval.removed.push_back(*it);
         } else {
             retval.updated.push_back(val);
@@ -142,11 +142,11 @@ VisibleMap::history(const vespalib::GenCnt& gen) const
     return retval;
 }
 
-VisibleMap::MapDiff::MapDiff() {}
-VisibleMap::MapDiff::~MapDiff() {}
+VisibleMap::MapDiff::MapDiff() = default;
+VisibleMap::MapDiff::~MapDiff() = default;
 
 VisibleMap::VisibleMap()
-    : _map(NULL),
+    : _map(nullptr),
       _waitList(),
       _genCnt(1)
 {
@@ -160,8 +160,8 @@ VisibleMap::~VisibleMap()
 bool
 VisibleMap::match(const char *name, const char *pattern)
 {
-    LOG_ASSERT(name != NULL);
-    LOG_ASSERT(pattern != NULL);
+    LOG_ASSERT(name != nullptr);
+    LOG_ASSERT(pattern != nullptr);
     while (*pattern != '\0') {
         if (*name == *pattern) {
             ++name;
diff --git a/slobrok/src/vespa/slobrok/server/visible_map.h b/slobrok/src/vespa/slobrok/server/visible_map.h
index 9ad6c13872e..0bd809bf4ef 100644
--- a/slobrok/src/vespa/slobrok/server/visible_map.h
+++ b/slobrok/src/vespa/slobrok/server/visible_map.h
@@ -56,9 +56,6 @@ private:
 
     static bool match(const char *name, const char *pattern);
 
-    VisibleMap(const VisibleMap &);            // Not used
-    VisibleMap &operator=(const VisibleMap &); // Not use
-
     void updated();
     void aborted();
 
@@ -80,6 +77,8 @@ public:
 
     MapDiff history(const vespalib::GenCnt& gen) const;
 
+    VisibleMap(const VisibleMap &) = delete;
+    VisibleMap &operator=(const VisibleMap &) = delete;
     VisibleMap();
     ~VisibleMap();
 };