[checkmk-commits] Windows agent: fix and clean Configurable.*

Module: check_mk Branch: master Commit: 3be1ad466f0d915f2096ae15b698a30013ff5d7e URL: http://git.mathias-kettner.de/git/?p=check_mk.git;a=commit;h=3be1ad466f0d91… Author: Jukka Aro &lt;ja(a)mathias-kettner.de&gt; Date: Wed Jan 24 13:19:06 2018 +0100 Windows agent: fix and clean Configurable.* * Declare base class destructors as virtual to ensure that derived class destructors get called for polymorphic instances. * Rely on implicitly declared and defined destructor where possible. * Manage polymorphic ConfigurableBase objects with RAII as unique_ptr's instead of raw pointers. * Remove empty source file Configurable.cc. --- agents/windows/Configurable.cc | 1 - agents/windows/Configurable.h | 12 +++++++++- agents/windows/Configuration.cc | 52 +++++++++++------------------------------ agents/windows/Configuration.h | 15 ++++++------ agents/windows/Makefile.am | 1 - agents/windows/build_version | 2 +- 6 files changed, 34 insertions(+), 49 deletions(-) diff --git a/agents/windows/Configurable.cc b/agents/windows/Configurable.cc deleted file mode 100644 index 71fd767..0000000 --- a/agents/windows/Configurable.cc +++ /dev/null @@ -1 +0,0 @@ -#include "Configurable.h" diff --git a/agents/windows/Configurable.h b/agents/windows/Configurable.h index 0db3412..2bb2fc1 100644 --- a/agents/windows/Configurable.h +++ b/agents/windows/Configurable.h @@ -13,6 +13,10 @@ class WinApiAdaptor; class ConfigurableBase { public: explicit ConfigurableBase(const WinApiAdaptor &winapi) : _winapi(winapi) {} + virtual ~ConfigurableBase() = default; + ConfigurableBase(const ConfigurableBase &) = delete; + ConfigurableBase &operator=(const ConfigurableBase &) = delete; + virtual void feed(const std::string &key, const std::string &value) = 0; virtual void output(const std::string &key, std::ostream &out) const = 0; virtual void startFile() = 0; @@ -31,7 +35,7 @@ public: config.reg(section, key, this); } - ~Configurable() {} + virtual ~Configurable() = default; ValueT *operator->() { return &_value; } @@ -75,6 +79,8 @@ public: config.reg(section, key, this); } + virtual ~ListConfigurable() = default; + ContainerT *operator->() { return &_values; } const ContainerT *operator->() const { return &_values; } @@ -156,6 +162,8 @@ public: config.reg(section, key, this); } + virtual ~KeyedListConfigurable() = default; + virtual void feed(const std::string &var, const std::string &value) override { size_t pos = var.find_first_of(" "); @@ -226,6 +234,8 @@ public: , _mapFunction(mapFunction) , _split_char(split_char) {} + virtual ~SplittingListConfigurable() = default; + virtual void feed(const std::string &key, const std::string &value) override { SuperT::clear(); diff --git a/agents/windows/Configuration.cc b/agents/windows/Configuration.cc index 92286cd..73679d0 100644 --- a/agents/windows/Configuration.cc +++ b/agents/windows/Configuration.cc @@ -35,48 +35,22 @@ #define __STDC_FORMAT_MACROS -Configuration::Configuration(const Environment &env) : _environment(env) {} - -Configuration::~Configuration() {} - void Configuration::readSettings() { - for (const auto &cfg : _configurables) { - for (auto entry : cfg.second) { - entry->startFile(); + for (bool local : {false, true}) { + for (const auto &cfg : _configurables) { + for (auto &entry : cfg.second) { + entry->startFile(); + } } - } - - readConfigFile(configFileName(false, _environment)); - for (const auto &cfg : _configurables) { - for (auto entry : cfg.second) { - entry->startFile(); - } + readConfigFile(configFileName(local, _environment)); } - - readConfigFile(configFileName(true, _environment)); } void Configuration::reg(const char *section, const char *key, ConfigurableBase *cfg) { _configurables[std::pair<std::string, std::string>(section, key)].push_back( - cfg); -} - -void Configuration::deregister(const char *section, const char *key, - ConfigurableBase *cfg) { - auto map_iter = - _configurables.find(std::pair<std::string, std::string>(section, key)); - if (map_iter != _configurables.end()) { - for (auto iter = map_iter->second.begin(); - iter != map_iter->second.end(); ++iter) { - if (cfg == *iter) { - map_iter->second.erase(iter); - break; - } - } - } - // if there is nothing to deregister that is actually a usage error + std::unique_ptr<ConfigurableBase>(cfg)); } std::string Configuration::configFileName(bool local, const Environment &env) { @@ -96,25 +70,27 @@ bool Configuration::checkHostRestriction(char *patterns) { } void Configuration::outputConfigurables(std::ostream &out) { - std::map<std::string, std::map<std::string, ConfigurableBase *>> config_map; + using ConfigMap = + std::map<std::string, std::reference_wrapper<ConfigurableBase>>; + std::map<std::string, ConfigMap> config_map; for (const auto &kv : _configurables) { std::string section, key; tie(section, key) = kv.first; if (config_map.find(section) == config_map.end()) { - config_map[section] = std::map<std::string, ConfigurableBase *>(); + config_map[section] = {}; } // this serializes only the first configurable registered under that // name, // if there are multiple with different mechanisms, this may be // confusing - config_map[section][key] = kv.second[0]; + config_map[section].emplace(key, *kv.second[0]); } for (const auto &section : config_map) { out << "[" << section.first << "]\n"; for (const auto &keys : section.second) { - keys.second->output(keys.first, out); + keys.second.get().output(keys.first, out); } } } @@ -182,7 +158,7 @@ void Configuration::readConfigFile(const std::string &filename) { auto map_iter = _configurables.find( config_key(section, std::string(variable, key_len))); if (map_iter != _configurables.end()) { - for (auto cfg : map_iter->second) { + for (auto &cfg : map_iter->second) { try { cfg->feed(variable, value); found = true; diff --git a/agents/windows/Configuration.h b/agents/windows/Configuration.h index 3a358d3..da9e3c7 100644 --- a/agents/windows/Configuration.h +++ b/agents/windows/Configuration.h @@ -26,6 +26,7 @@ #define Configuration_h #include <map> +#include <memory> #include "SettingsCollector.h" class ConfigurableBase; @@ -61,22 +62,23 @@ check = DISK_C: mrpe/check_disk -w C: check = MEM mrpe/check_mem -w 10 -c 20 */ class Configuration { - typedef std::pair<std::string, std::string> ConfigKey; + using ConfigKey = std::pair<std::string, std::string>; + using ConfigurableKey = std::pair<std::string, std::string>; + using ConfigurableMap = + std::map<ConfigurableKey, + std::vector<std::unique_ptr<ConfigurableBase>>>; ConfigKey config_key(const std::string &section, const std::string &key) { return std::make_pair(section, key); } public: - Configuration(const Environment &env); - ~Configuration(); + Configuration(const Environment &env) : _environment(env) {} void outputConfigurables(std::ostream &out); void readSettings(); void reg(const char *section, const char *key, ConfigurableBase *cfg); - void deregister(const char *section, const char *key, - ConfigurableBase *cfg); inline const Environment &getEnvironment() const { return _environment; } static std::string configFileName(bool local, const Environment &env); @@ -87,8 +89,7 @@ private: bool checkHostRestriction(char *patterns); private: - typedef std::pair<std::string, std::string> ConfigurableKey; - std::map<ConfigurableKey, std::vector<ConfigurableBase *>> _configurables; + ConfigurableMap _configurables; const Environment &_environment; }; diff --git a/agents/windows/Makefile.am b/agents/windows/Makefile.am index 1ba5199..e51f84b 100644 --- a/agents/windows/Makefile.am +++ b/agents/windows/Makefile.am @@ -34,7 +34,6 @@ unittest: WindowsAgentTest$(EXEEXT) DISPLAY="$$DISPLAY xterm" wine WindowsAgentTest$(EXEEXT) 2>/dev/null libcheck_mk_agent_a_SOURCES = \ - Configurable.cc \ Configuration.cc \ CrashHandler.cc \ Crypto.cc \ diff --git a/agents/windows/build_version b/agents/windows/build_version index ac7e0e6..311e81e 100644 --- a/agents/windows/build_version +++ b/agents/windows/build_version @@ -1 +1 @@ -3078 +3080