Replace config visit pattern with explicit lookups

This is an initial pass at doing explicit value checks when handling
config parsing, as opposed to using a visiting pattern. The latter
made it difficult to check for conditions such as missing required
values, multiple values, etc.

It was also generally less readable (think declarative) which further
made it difficult to get a grasp for what our actual configuration file
requirements were.

llarp/config/config.cpp

@@ -5,6 +5,7 @@
 #include <constants/limits.hpp>
 #include <net/net.hpp>
 #include <router_contact.hpp>
+#include <stdexcept>
 #include <util/fs.hpp>
 #include <util/logging/logger_syslog.hpp>
 #include <util/logging/logger.hpp>
@@ -92,138 +93,171 @@ namespace llarp
     return {};
   }
 
-  void
-  RouterConfig::fromSection(string_view key, string_view val)
+  bool
+  RouterConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "job-queue-size")
-    {
-      auto sval = svtoi(val);
-      if (sval >= 1024)
-      {
-        m_JobQueueSize = sval;
-        LogInfo("Set job queue size to ", m_JobQueueSize);
-      }
-    }
-    if (key == "default-protocol")
+    // [router]:job-queue-size
+    auto parsedValue = parser.getSingleSectionValue(values, "router", "job-queue-size", false);
+    if (not parsedValue.empty()) 
     {
-      m_DefaultLinkProto = str(val);
-      LogInfo("overriding default link protocol to '", val, "'");
-    }
-    if (key == "netid")
-    {
-      if (val.size() <= NetID::size())
-      {
-        m_netId = str(val);
-        LogInfo("setting netid to '", val, "'");
-      }
+      int val = svtoi(parsedValue);
+      if (val < 1024)
+        throw std::invalid_argument("invalid value for [router]:job-queue-size, must be 1024 or greater");
       else
-      {
-        llarp::LogError("invalid netid '", val, "', is too long");
-      }
+        m_JobQueueSize = val;
     }
-    if (key == "max-connections")
+
+    // [router]:default-protocol
+    parsedValue = parser.getSingleSectionValue(values, "router", "default-protocol", false);
+    if (not parsedValue.empty())
+      m_DefaultLinkProto = parsedValue;
+
+    // [router]:netid
+    parsedValue = parser.getSingleSectionValue(values, "router", "netid", true);
+    assert(not parsedValue.empty()); // gauranteed by getSingleSectionValue() with required == true
+    if(parsedValue.size() > NetID::size())
+      throw std::invalid_argument("value for [router]:netid is too long");
+    m_netId = str(parsedValue);
+
+    // [router]:max-connections
+    parsedValue = parser.getSingleSectionValue(values, "router", "max-connections", false);
+    if (not parsedValue.empty())
     {
-      auto ival = svtoi(val);
-      if (ival > 0)
-      {
-        m_maxConnectedRouters = ival;
-        LogInfo("max connections set to ", m_maxConnectedRouters);
-      }
+      int val = svtoi(parsedValue);
+      if (val < 1)
+        throw std::invalid_argument("invalid value for [router]:max-connections");
+      else
+        m_maxConnectedRouters = val;
     }
-    if (key == "min-connections")
+
+    // [router]:min-connections
+    parsedValue = parser.getSingleSectionValue(values, "router", "min-connections", false);
+    if (not parsedValue.empty())
     {
-      auto ival = svtoi(val);
-      if (ival > 0)
-      {
-        m_minConnectedRouters = ival;
-        LogInfo("min connections set to ", m_minConnectedRouters);
-      }
+      int val = svtoi(parsedValue);
+      if (val < 1)
+        throw std::invalid_argument("invalid value for [router]:min-connections");
+      else
+        m_minConnectedRouters = val;
     }
-    if (key == "nickname")
+
+    // additional check that min <= max
+    if (m_minConnectedRouters > m_maxConnectedRouters)
+      throw std::invalid_argument("[router]:min-connections must be less than [router]:max-connections");
+
+    // [router]:nickname
+    parsedValue = parser.getSingleSectionValue(values, "router", "nickname", false);
+    if (not parsedValue.empty())
     {
-      m_nickname = str(val);
-      // set logger name here
+      m_nickname = str(parsedValue);
+      // TODO: side effect here, no side effects in config parsing!!
       LogContext::Instance().nodeName = nickname();
-      LogInfo("nickname set");
-    }
-    if (key == "encryption-privkey")
-    {
-      m_encryptionKeyfile = str(val);
-      LogDebug("encryption key set to ", m_encryptionKeyfile);
-    }
-    if (key == "contact-file")
-    {
-      m_ourRcFile = str(val);
-      LogDebug("rc file set to ", m_ourRcFile);
-    }
-    if (key == "transport-privkey")
-    {
-      m_transportKeyfile = str(val);
-      LogDebug("transport key set to ", m_transportKeyfile);
     }
-    if ((key == "identity-privkey" || key == "ident-privkey"))
-    {
-      m_identKeyfile = str(val);
-      LogDebug("identity key set to ", m_identKeyfile);
-    }
-    if (key == "public-address" || key == "public-ip")
-    {
-      llarp::LogInfo("public ip ", val, " size ", val.size());
-      if (val.size() < 17)
+
+    // [router]:encryption-privkey
+    parsedValue = parser.getSingleSectionValue(values, "router", "encryption-privkey", false);
+    if (not parsedValue.empty())
+      m_encryptionKeyfile = str(parsedValue);
+
+    // [router]:contact-file
+    parsedValue = parser.getSingleSectionValue(values, "router", "contact-file", false);
+    if (not parsedValue.empty())
+      m_ourRcFile = str(parsedValue);
+
+    // [router]:transport-privkey
+    parsedValue = parser.getSingleSectionValue(values, "router", "transport-privkey", false);
+    if (not parsedValue.empty())
+      m_transportKeyfile = str(parsedValue);
+
+    // [router]:identity-privkey OR
+    // [router]:ident-privkey
+    // apparently loki-launcher made its own config files at one point and typoed this,
+    // so we support both
+    parsedValue = parser.getSingleSectionValue(values, "router", "identity-privkey", false);
+    if (parsedValue.empty()) 
+      parsedValue = parser.getSingleSectionValue(values, "router", "ident-privkey", false);
+    if (not parsedValue.empty())
+      m_identKeyfile = str(parsedValue);
+
+    // [router]:public-address OR
+    // [router]:public-ip
+    // apparently loki-launcher made its own config files at one point and typoed this,
+    // so we support both
+    parsedValue = parser.getSingleSectionValue(values, "router", "public-address", false);
+    if (parsedValue.empty()) 
+      parsedValue = parser.getSingleSectionValue(values, "router", "public-ip", false);
+    if (not parsedValue.empty())
+    {
+      llarp::LogInfo("public ip ", parsedValue, " size ", parsedValue.size());
+      if(parsedValue.size() < 17)
       {
         // assume IPv4
-        llarp::Addr a(val);
+        llarp::Addr a(parsedValue);
         llarp::LogInfo("setting public ipv4 ", a);
         m_addrInfo.ip = *a.addr6();
         m_publicOverride = true;
       }
     }
-    if (key == "public-port")
+
+    // [router]:public-port
+    parsedValue = parser.getSingleSectionValue(values, "router", "public-port", false);
+    if (not parsedValue.empty()) 
     {
-      llarp::LogInfo("Setting public port ", val);
-      int p = svtoi(val);
+      llarp::LogInfo("Setting public port ", parsedValue);
+      int p = svtoi(parsedValue);
       // Not needed to flip upside-down - this is done in llarp::Addr(const
       // AddressInfo&)
       m_ip4addr.sin_port = p;
       m_addrInfo.port = p;
       m_publicOverride = true;
     }
-    if (key == "worker-threads" || key == "threads")
+
+    // [router]:worker-threads OR
+    // [router]:threads
+    // apparently loki-launcher made its own config files at one point and typoed this,
+    // so we support both
+    parsedValue = parser.getSingleSectionValue(values, "router", "worker-threads", false);
+    if (parsedValue.empty()) 
+      parsedValue = parser.getSingleSectionValue(values, "router", "threads", false);
+    if (not parsedValue.empty())
     {
-      m_workerThreads = svtoi(val);
-      if (m_workerThreads <= 0)
-      {
-        LogWarn("worker threads invalid value: '", val, "' defaulting to 1");
-        m_workerThreads = 1;
-      }
+      int val = svtoi(parsedValue);
+      if(val <= 0)
+        throw std::invalid_argument("invalid value for [router]:worker-threads");
       else
-      {
-        LogDebug("set to use ", m_workerThreads, " worker threads");
-      }
+        m_workerThreads = val;
     }
-    if (key == "net-threads")
+
+    // [router]:public-port
+    parsedValue = parser.getSingleSectionValue(values, "router", "public-port", false);
+    if (not parsedValue.empty())
     {
-      m_numNetThreads = svtoi(val);
-      if (m_numNetThreads <= 0)
-      {
-        LogWarn("net threads invalid value: '", val, "' defaulting to 1");
-        m_numNetThreads = 1;
-      }
+      int val = svtoi(parsedValue);
+      if (val <= 0)
+        throw std::invalid_argument("invalid value for [router]:public-port");
       else
-      {
-        LogDebug("set to use ", m_numNetThreads, " net threads");
-      }
+        m_numNetThreads = val;
     }
-    if (key == "block-bogons")
+
+    // [router]:block-bogons
+    parsedValue = parser.getSingleSectionValue(values, "router", "block-bogons", false);
+    if (not parsedValue.empty())
     {
-      m_blockBogons = setOptBool(val);
+      auto val = setOptBool(parsedValue);
+      if (not val.has_value())
+        throw std::invalid_argument("invalid value for [router]:block-bogons");
+      else
+        m_blockBogons = val;
     }
+
+    return true;
   }
 
-  void
-  NetworkConfig::fromSection(string_view key, string_view val)
+  bool
+  NetworkConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "profiling")
+    /*
+    if(key == "profiling")
     {
       m_enableProfiling = setOptBool(val);
     }
@@ -240,21 +274,28 @@ namespace llarp
     {
       m_netConfig.emplace(str(key), str(val));  // str()'s here for gcc 5 compat
     }
+    */
+
+    return true;
   }
 
-  void
-  NetdbConfig::fromSection(string_view key, string_view val)
+  bool
+  NetdbConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "dir")
+    /*
+    if(key == "dir")
     {
       m_nodedbDir = str(val);
     }
+    */
+    return true;
   }
 
-  void
-  DnsConfig::fromSection(string_view key, string_view val)
+  bool
+  DnsConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "upstream")
+    /*
+    if(key == "upstream")
     {
       llarp::LogInfo("add upstream resolver ", val);
       netConfig.emplace("upstream-dns", str(val));  // str() for gcc 5 compat
@@ -264,11 +305,14 @@ namespace llarp
       llarp::LogInfo("set local dns to ", val);
       netConfig.emplace("local-dns", str(val));  // str() for gcc 5 compat
     }
+    */
+    return true;
   }
 
-  void
-  LinksConfig::fromSection(string_view key, string_view val)
+  bool
+  LinksConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
+    /*
     uint16_t proto = 0;
 
     std::unordered_set<std::string> parsed_opts;
@@ -317,33 +361,41 @@ namespace llarp
       // str() here for gcc 5 compat
       m_InboundLinks.emplace_back(str(key), AF_INET, proto, std::move(opts));
     }
+    */
+    return true;
   }
 
-  void
-  ConnectConfig::fromSection(string_view /*key*/, string_view val)
+  bool
+  ConnectConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    routers.emplace_back(val.begin(), val.end());
+    // routers.emplace_back(val.begin(), val.end());
+    return true;
   }
 
-  void
-  ServicesConfig::fromSection(string_view key, string_view val)
+  bool
+  ServicesConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    services.emplace_back(str(key), str(val));  // str()'s here for gcc 5 compat
+    // services.emplace_back(str(key), str(val));  // str()'s here for gcc 5 compat
+    return true;
   }
 
-  void
-  SystemConfig::fromSection(string_view key, string_view val)
+  bool
+  SystemConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "pidfile")
+    /*
+    if(key == "pidfile")
     {
       pidfile = str(val);
     }
+    */
+    return true;
   }
 
-  void
-  ApiConfig::fromSection(string_view key, string_view val)
+  bool
+  ApiConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "enabled")
+    /*
+    if(key == "enabled")
     {
       m_enableRPCServer = IsTrueValue(val);
     }
@@ -355,12 +407,15 @@ namespace llarp
     {
       // TODO: add pubkey to whitelist
     }
+    */
+    return true;
   }
 
-  void
-  LokidConfig::fromSection(string_view key, string_view val)
+  bool
+  LokidConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "service-node-seed")
+    /*
+    if(key == "service-node-seed")
     {
       usingSNSeed = true;
       ident_keyfile = std::string{val};
@@ -381,21 +436,27 @@ namespace llarp
     {
       lokidRPCPassword = str(val);
     }
+    */
+    return true;
   }
 
-  void
-  BootstrapConfig::fromSection(string_view key, string_view val)
+  bool
+  BootstrapConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "add-node")
+    /*
+    if(key == "add-node")
     {
       routers.emplace_back(val.begin(), val.end());
     }
+    */
+    return true;
   }
 
-  void
-  LoggingConfig::fromSection(string_view key, string_view val)
+  bool
+  LoggingConfig::parseSectionValues(const ConfigParser& parser, const SectionValues_t& values)
   {
-    if (key == "type" && val == "syslog")
+    /*
+    if(key == "type" && val == "syslog")
     {
       // TODO(despair): write event log syslog class
 #if defined(_WIN32)
@@ -443,25 +504,26 @@ namespace llarp
         ::abort();
       }
     }
+    */
+    return true;
   }
 
   template < typename Section >
   Section
   find_section(const ConfigParser &parser, const std::string &name)
   {
-    Section ret;
+    Section section;
 
-    auto visitor = [&ret](const ConfigParser::Section_t& section) -> bool {
-      for (const auto& sec : section)
-      {
-        ret.fromSection(sec.first, sec.second);
-      }
-      return true;
+    auto visitor = [&](const ConfigParser::SectionValues_t& sectionValues) {
+      return section.parseSectionValues(parser, sectionValues);
     };
 
+    // TODO: exceptions, please. fuck.
+    //       parser.VisitSection just passes-through the return value of our
+    //       lambda from above
     if(parser.VisitSection(name.c_str(), visitor))
     {
-      return ret;
+      return section;
     }
 
     return {};

llarp/config/config.hpp

@@ -5,6 +5,7 @@
 #include <router_contact.hpp>
 #include <util/fs.hpp>
 #include <util/str.hpp>
+#include <config/ini.hpp>
 
 #include <cstdlib>
 #include <functional>
@@ -16,7 +17,8 @@
 struct llarp_config;
 namespace llarp
 {
-  struct ConfigParser;
+  using SectionValues_t = llarp::ConfigParser::SectionValues_t;
+  using Config_impl_t = llarp::ConfigParser::Config_impl_t;
 
   inline const char*
   lokinetEnv(string_view suffix);
@@ -88,8 +90,8 @@ namespace llarp
     nonstd::optional< bool > blockBogons() const { return fromEnv(m_blockBogons, "BLOCK_BOGONS"); }
     // clang-format on
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   class NetworkConfig
@@ -111,8 +113,8 @@ namespace llarp
     const NetConfig& netConfig() const             { return m_netConfig; }
     // clang-format on
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   class NetdbConfig
@@ -125,16 +127,16 @@ namespace llarp
     std::string nodedbDir() const { return fromEnv(m_nodedbDir, "NODEDB_DIR"); }
     // clang-format on
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct DnsConfig
   {
     std::unordered_multimap<std::string, std::string> netConfig;
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   class LinksConfig
@@ -160,31 +162,31 @@ namespace llarp
     const Links& inboundLinks() const { return m_InboundLinks; }
     // clang-format on
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct ConnectConfig
   {
     std::vector<std::string> routers;
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct ServicesConfig
   {
-    std::vector<std::pair<std::string, std::string>> services;
-    void
-    fromSection(string_view key, string_view val);
+    std::vector< std::pair< std::string, std::string > > services;
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct SystemConfig
   {
     std::string pidfile;
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   class ApiConfig
@@ -199,8 +201,8 @@ namespace llarp
     std::string rpcBindAddr() const { return fromEnv(m_rpcBindAddr, "RPC_BIND_ADDR"); }
     // clang-format on
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct LokidConfig
@@ -212,15 +214,15 @@ namespace llarp
     std::string lokidRPCUser;
     std::string lokidRPCPassword;
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct BootstrapConfig
   {
-    std::vector<std::string> routers;
-    void
-    fromSection(string_view key, string_view val);
+    std::vector< std::string > routers;
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct LoggingConfig
@@ -228,8 +230,8 @@ namespace llarp
     bool m_LogJSON = false;
     FILE* m_LogFile = stdout;
 
-    void
-    fromSection(string_view key, string_view val);
+    bool
+    parseSectionValues(const ConfigParser& parser, const SectionValues_t& values);
   };
 
   struct Config

llarp/config/ini.cpp

@@ -1,11 +1,13 @@
 #include <config/ini.hpp>
 
 #include <util/logging/logger.hpp>
+#include <util/str.hpp>
 
 #include <cctype>
 #include <fstream>
 #include <list>
 #include <iostream>
+#include <cassert>
 
 namespace llarp
 {
@@ -129,7 +131,7 @@ namespace llarp
           LogError(m_FileName, " invalid line (", lineno, "): '", line, "'");
           return false;
         }
-        Section_t& sect = m_Config[str(sectName)];
+        SectionValues_t& sect = m_Config[str(sectName)];
         LogDebug(m_FileName, ": ", sectName, ".", k, "=", v);
         sect.emplace(str(k), str(v));  // str()'s here for gcc 5 compat
       }
@@ -143,16 +145,56 @@ namespace llarp
   }
 
   void
-  ConfigParser::IterAll(std::function<void(string_view, const Section_t&)> visit)
+  ConfigParser::IterAll(
+      std::function< void(string_view, const SectionValues_t&) > visit)
   {
     for (const auto& item : m_Config)
       visit(item.first, item.second);
   }
 
+  const std::string emptyStr = "";
+
+  const std::string&
+  ConfigParser::getSingleSectionValue(
+      const SectionValues_t& values,
+      const std::string& section,
+      const std::string& key,
+      bool required,
+      bool tolerateMultiples) const
+  {
+    // section is provided for clarity / sanity check
+    assert(m_Config.find(section) != m_Config.end());
+    assert( & m_Config.find(section)->second ==  &values);
+
+    const auto sectionItr = m_Config.find(section);
+    if (sectionItr == m_Config.end())
+      throw std::invalid_argument(stringify("could not find section ", section));
+
+    const SectionValues_t& sectionValues = sectionItr->second;
+
+    if (not tolerateMultiples and sectionValues.count(key) > 1)
+      throw std::invalid_argument(stringify("config [", section, "]:", key, " cannot have multiple values for ", key));
+
+    const auto itr = sectionValues.find(key);
+    if (itr == sectionValues.end()) {
+      if (required)
+        throw std::invalid_argument(stringify("config [", section, "]:", key, " is missing"));
+      else
+        return emptyStr;
+    }
+
+    return itr->second;
+  }
+
   bool
   ConfigParser::VisitSection(
-      const char* name, std::function<bool(const Section_t& sect)> visit) const
+      const char* name,
+      std::function< bool(const SectionValues_t& sect) > visit) const
   {
+    // m_Config is effectively:
+    // unordered_map< string, unordered_multimap< string, string  >>
+    // in human terms: a map of of sections
+    //                 where a section is a multimap of k:v pairs
     auto itr = m_Config.find(name);
     if (itr == m_Config.end())
       return false;

llarp/config/ini.hpp

@@ -12,8 +12,8 @@ namespace llarp
 {
   struct ConfigParser
   {
-    using Section_t = std::unordered_multimap<std::string, std::string>;
-    using Config_impl_t = std::unordered_map<std::string, Section_t>;
+    using SectionValues_t = std::unordered_multimap< std::string, std::string >;
+    using Config_impl_t = std::unordered_map< std::string, SectionValues_t >;
     /// clear parser
     void
     Clear();
@@ -32,12 +32,35 @@ namespace llarp
 
     /// iterate all sections and thier values
     void
-    IterAll(std::function<void(string_view, const Section_t&)> visit);
+    IterAll(std::function< void(string_view, const SectionValues_t&) > visit);
 
     /// visit a section in config read only by name
     /// return false if no section or value propagated from visitor
     bool
-    VisitSection(const char* name, std::function<bool(const Section_t&)> visit) const;
+    VisitSection(const char* name,
+                 std::function< bool(const SectionValues_t&) > visit) const;
+
+    /// Obtain a section value for the given key, additionally imposing the
+    /// provided constraints. an `invalid_argument` will be thrown if the
+    /// constraints aren't met.
+    /// 
+    /// The `section` parameter is redundant and added for readability, but a call to
+    /// m_Config[section] should result in the same object as `values`.
+    /// 
+    /// @param values is the SectionValues map in which to search for values
+    /// @param section should correspond to INI section tag related to this config
+    /// @param key is the key to look up
+    /// @param bool constrains whether this key must exist
+    /// @param tolerateMultiples constrains whether multiples are allowed
+    /// @return the first matching entry if found or empty string if not found
+    /// @throws std::invalid_argument if constrants aren't met or if `section` is not found
+    const std::string&
+    getSingleSectionValue(
+        const SectionValues_t& values,
+        const std::string& section,
+        const std::string& key,
+        bool required,
+        bool tolerateMultiples = false) const;
 
    private:
     bool

llarp/service/config.cpp

@@ -12,13 +12,14 @@ namespace llarp
       ConfigParser parser;
       if (!parser.LoadFile(fname))
         return false;
-      parser.IterAll([&](string_view name, const ConfigParser::Section_t& section) {
-        Config::section_t values;
-        values.first.assign(name.begin(), name.end());
-        for (const auto& item : section)
-          values.second.emplace_back(item.first, item.second);
-        services.emplace_back(values);
-      });
+      parser.IterAll(
+          [&](string_view name, const ConfigParser::SectionValues_t& section) {
+            Config::section_t values;
+            values.first.assign(name.begin(), name.end());
+            for(const auto& item : section)
+              values.second.emplace_back(item.first, item.second);
+            services.emplace_back(values);
+          });
       return services.size() > 0;
     }
 

test/config/test_llarp_config_ini.cpp

@@ -20,7 +20,7 @@ TEST_F(TestINIParser, TestParseEmpty)
 
 TEST_F(TestINIParser, TestParseOneSection)
 {
-  llarp::ConfigParser::Section_t sect;
+  llarp::ConfigParser::SectionValues_t sect;
   // this is an anti pattern don't write this kind of code with configpaser
   auto assertVisit = [&sect](const auto& section) -> bool {
     sect = section;

test/util/test_llarp_util_log_level.cpp

@@ -63,36 +63,3 @@ TEST_F(LogLevelTest, TestLogLevelToString)
   EXPECT_EQ("???", LogLevelToString(llarp::eLogNone));
 }
 
-TEST_F(LogLevelTest, TestLoggingConfigSideEffects)
-{
-  // restore original runtime level when we're done
-  llarp::LogContext& logContext = llarp::LogContext::Instance();
-  auto original = logContext.runtimeLevel;
-
-  // LoggingConfig::fromSection updates the runtime level as it reads in conf
-  // values, so feed it values and ensure that the runtime level is updated
-  // appropriately
-  llarp::LoggingConfig config;
-
-  config.fromSection("level", "Trace");
-  EXPECT_EQ(llarp::eLogTrace, logContext.runtimeLevel);
-
-  config.fromSection("level", "Debug");
-  EXPECT_EQ(llarp::eLogDebug, logContext.runtimeLevel);
-
-  config.fromSection("level", "Info");
-  EXPECT_EQ(llarp::eLogInfo, logContext.runtimeLevel);
-
-  config.fromSection("level", "Warn");
-  EXPECT_EQ(llarp::eLogWarn, logContext.runtimeLevel);
-
-  config.fromSection("level", "Error");
-  EXPECT_EQ(llarp::eLogError, logContext.runtimeLevel);
-
-  config.fromSection("level", "None");
-  EXPECT_EQ(llarp::eLogNone, logContext.runtimeLevel);
-
-
-  SetLogLevel(original);
-}
-