Rewrite the large ClusterState constructor that gcc 7 compiled incorrectly.

Probably still an issue with gcc7, but now the code is a bit more readable.

2 files changed, 121 insertions, 126 deletions

diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.cpp b/vdslib/src/vespa/vdslib/state/clusterstate.cpp
index 7ca91788200..61272d8a3d3 100644
--- a/vdslib/src/vespa/vdslib/state/clusterstate.cpp
+++ b/vdslib/src/vespa/vdslib/state/clusterstate.cpp
@@ -3,14 +3,16 @@
 
 #include <vespa/vespalib/text/stringtokenizer.h>
 #include <vespa/document/util/stringutil.h>
-#include <sstream>
 #include <vespa/vdslib/distribution/distribution.h>
 #include <vespa/vespalib/util/exceptions.h>
 #include <vespa/vespalib/stllike/asciistream.h>
-#include <vespa/log/log.h>
+#include <sstream>
 
+#include <vespa/log/log.h>
 LOG_SETUP(".vdslib.state.cluster");
 
+using vespalib::IllegalArgumentException;
+
 namespace storage {
 namespace lib {
 
@@ -22,51 +24,36 @@ ClusterState::ClusterState()
       _nodeCount(2),
       _description(),
       _distributionBits(16)
-{
-}
-
-ClusterState::ClusterState(const ClusterState& other)
-    : Printable(other),
-      _version(other._version),
-      _clusterState(other._clusterState),
-      _nodeStates(other._nodeStates),
-      _nodeCount(other._nodeCount),
-      _description(other._description),
-      _distributionBits(other._distributionBits)
-{
-}
+{ }
 
-ClusterState::~ClusterState()
-{
-}
+ClusterState::ClusterState(const ClusterState& other) = default;
+ClusterState::~ClusterState() { }
 
-namespace {
-    struct NodeData {
-        bool empty;
-        Node node;
-        vespalib::asciistream ost;
+struct NodeData {
+    bool empty;
+    Node node;
+    vespalib::asciistream ost;
 
-        NodeData() : empty(true), node(NodeType::STORAGE, 0), ost() {}
+    NodeData() : empty(true), node(NodeType::STORAGE, 0), ost() {}
 
-        void addTo(std::map<Node, NodeState>& nodeStates,
-                   std::vector<uint16_t>& nodeCount)
-        {
-            if (!empty) {
-                NodeState state(ost.str(), &node.getType());
-                if (state != NodeState(node.getType(), State::UP)
-                    || state.getDescription().size() > 0)
-                {
-                    nodeStates.insert(std::make_pair(node, state));
-                }
-                if (nodeCount[node.getType()] <= node.getIndex()) {
-                    nodeCount[node.getType()] = node.getIndex() + 1;
-                }
-                empty = true;
-                ost.clear();
+    void addTo(std::map<Node, NodeState>& nodeStates,
+               std::vector<uint16_t>& nodeCount)
+    {
+        if (!empty) {
+            NodeState state(ost.str(), &node.getType());
+            if (state != NodeState(node.getType(), State::UP)
+                || state.getDescription().size() > 0)
+            {
+                nodeStates.insert(std::make_pair(node, state));
             }
+            if (nodeCount[node.getType()] <= node.getIndex()) {
+                nodeCount[node.getType()] = node.getIndex() + 1;
+            }
+            empty = true;
+            ost.clear();
         }
-    };
-}
+    }
+};
 
 ClusterState::ClusterState(const vespalib::stringref & serialized)
     : Printable(),
@@ -82,108 +69,113 @@ ClusterState::ClusterState(const vespalib::stringref & serialized)
     NodeData nodeData;
     vespalib::string lastAbsolutePath;
 
-    for (vespalib::StringTokenizer::Iterator it = st.begin();
-         it != st.end(); ++it)
-    {
+    for (vespalib::StringTokenizer::Iterator it = st.begin(); it != st.end(); ++it) {
         vespalib::string::size_type index = it->find(':');
         if (index == vespalib::string::npos) {
-            throw vespalib::IllegalArgumentException(
-                    "Token " + *it + " does not contain ':': " + serialized,
-                    VESPA_STRLOC);
+            throw IllegalArgumentException("Token " + *it + " does not contain ':': " + serialized, VESPA_STRLOC);
         }
         vespalib::stringref key = it->substr(0, index);
         vespalib::stringref value = it->substr(index + 1);
         if (key.size() > 0 && key[0] == '.') {
             if (lastAbsolutePath == "") {
-                throw vespalib::IllegalArgumentException(
-                        "The first path in system state string needs to be "
-                        "absolute", VESPA_STRLOC);
+                throw IllegalArgumentException("The first path in system state string needs to be absolute", VESPA_STRLOC);
             }
             key = lastAbsolutePath + key;
         } else {
             lastAbsolutePath = key;
         }
-        if (key.size() > 0) switch (key[0]) {
-        case 'c':
-            if (key == "cluster") {
-                setClusterState(State::get(value));
-                continue;
-            }
-            break;
-        case 'b':
-            if (key == "bits") {
-                _distributionBits = atoi(value.c_str());
-                continue;
-            }
-            break;
-        case 'v':
-            if (key == "version") {
-                _version = atoi(value.c_str());
-                continue;
-            }
-            break;
-        case 'm':
-            if (key.size() > 1) break;
+        
+        if (key.empty() || parse(key, value, nodeData) ) {
+            LOG(debug, "Unknown key %s in systemstate. Ignoring it, assuming it's "
+                       "a new feature from a newer version than ourself: %s",
+                       key.c_str(), serialized.c_str());
+        }
+    }
+    nodeData.addTo(_nodeStates, _nodeCount);
+    removeExtraElements();
+}
+
+bool
+ClusterState::parse(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData) {
+    switch (key[0]) {
+    case 'c':
+        if (key == "cluster") {
+            setClusterState(State::get(value));
+            return true;
+        }
+        break;
+    case 'b':
+        if (key == "bits") {
+            _distributionBits = atoi(value.c_str());
+            return true;
+        }
+        break;
+    case 'v':
+        if (key == "version") {
+            _version = atoi(value.c_str());
+            return true;
+        }
+        break;
+    case 'm':
+        if (key.size() == 1) {
             _description = document::StringUtil::unescape(value);
-            continue;
-        case 'd':
-        case 's':
-        {
-            const NodeType* nodeType(0);
-            vespalib::string::size_type dot = key.find('.');
-            vespalib::stringref type(dot == vespalib::string::npos
-                             ? key : key.substr(0, dot));
-            if (type == "storage") {
-                nodeType = &NodeType::STORAGE;
-            } else if (type == "distributor") {
-                nodeType = &NodeType::DISTRIBUTOR;
-            }
-            if (nodeType == 0) break;
-            if (dot == vespalib::string::npos) { // Entry that set node counts
-                uint16_t nodeCount = 0;
-                nodeCount = atoi(value.c_str());
+            return true;
+        }
+        break;
+    case 'd':
+    case 's':
+        return parseSorD(key, value, nodeData);
+    default:
+        break;
+    }
+    return false;
+}
 
-                if (nodeCount > _nodeCount[*nodeType] ) {
-                    _nodeCount[*nodeType] = nodeCount;
-                }
-                continue;
-            }
-            vespalib::string::size_type dot2 = key.find('.', dot + 1);
-            Node node;
-            if (dot2 == vespalib::string::npos) {
-                node = Node(*nodeType, atoi(key.substr(dot + 1).c_str()));
-            } else {
-                node = Node(*nodeType, atoi(key.substr(dot + 1, dot2 - dot - 1).c_str()));
-            }
+bool
+ClusterState::parseSorD(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData) {
+    const NodeType* nodeType(0);
+    vespalib::string::size_type dot = key.find('.');
+    vespalib::stringref type(dot == vespalib::string::npos
+                             ? key : key.substr(0, dot));
+    if (type == "storage") {
+        nodeType = &NodeType::STORAGE;
+    } else if (type == "distributor") {
+        nodeType = &NodeType::DISTRIBUTOR;
+    }
+    if (nodeType == 0) return false;
+    if (dot == vespalib::string::npos) { // Entry that set node counts
+        uint16_t nodeCount = 0;
+        nodeCount = atoi(value.c_str());
 
-            if (node.getIndex() >= _nodeCount[*nodeType]) {
-                vespalib::asciistream ost;
-                ost << "Cannot index " << *nodeType << " node "
-                    << node.getIndex() << " of " << _nodeCount[*nodeType];
-                throw vespalib::IllegalArgumentException(
-                        ost.str(), VESPA_STRLOC);
-            }
-            if (nodeData.node != node) {
-                nodeData.addTo(_nodeStates, _nodeCount);
-            }
-            if (dot2 == vespalib::string::npos) {
-                break; // No default key for nodes.
-            } else {
-                nodeData.ost << " " << key.substr(dot2 + 1) << ':' << value;
-            }
-            nodeData.node = node;
-            nodeData.empty = false;
-            continue;
-        }
-        default:
-            break;
+        if (nodeCount > _nodeCount[*nodeType] ) {
+            _nodeCount[*nodeType] = nodeCount;
         }
-        LOG(debug, "Unknown key %s in systemstate. Ignoring it, assuming it's "
-            "a new feature from a newer version than ourself: %s",
-            key.c_str(), serialized.c_str());
+        return true;
     }
-    nodeData.addTo(_nodeStates, _nodeCount);
-    removeExtraElements();
+    vespalib::string::size_type dot2 = key.find('.', dot + 1);
+    Node node;
+    if (dot2 == vespalib::string::npos) {
+        node = Node(*nodeType, atoi(key.substr(dot + 1).c_str()));
+    } else {
+        node = Node(*nodeType, atoi(key.substr(dot + 1, dot2 - dot - 1).c_str()));
+    }
+
+    if (node.getIndex() >= _nodeCount[*nodeType]) {
+        vespalib::asciistream ost;
+        ost << "Cannot index " << *nodeType << " node " << node.getIndex() << " of " << _nodeCount[*nodeType];
+        throw IllegalArgumentException( ost.str(), VESPA_STRLOC);
+    }
+    if (nodeData.node != node) {
+        nodeData.addTo(_nodeStates, _nodeCount);
+    }
+    if (dot2 == vespalib::string::npos) {
+        return false; // No default key for nodes.
+    } else {
+        nodeData.ost << " " << key.substr(dot2 + 1) << ':' << value;
+    }
+    nodeData.node = node;
+    nodeData.empty = false;
+    return true;
 }
 
 namespace {
diff --git a/vdslib/src/vespa/vdslib/state/clusterstate.h b/vdslib/src/vespa/vdslib/state/clusterstate.h
index dcea1308b6b..b60273e6913 100644
--- a/vdslib/src/vespa/vdslib/state/clusterstate.h
+++ b/vdslib/src/vespa/vdslib/state/clusterstate.h
@@ -17,6 +17,7 @@ namespace lib {
 
 class Distribution;
 class Group;
+class NodeData;
 
 class ClusterState : public document::Printable {
     uint32_t _version;
@@ -70,6 +71,8 @@ public:
                              const std::string& indent = "") const;
 
 private:
+    bool parse(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData);
+    bool parseSorD(vespalib::stringref key, vespalib::stringref value, NodeData & nodeData);
     void removeExtraElements();
     void printStateGroupwise(std::ostream& out, const Group&, bool verbose,
                              const std::string& indent, bool rootGroup) const;