From 26378606b9b2d1d408033712a193c5cbccf4e2d6 Mon Sep 17 00:00:00 2001 From: Jan Becker Date: Wed, 29 May 2024 09:40:04 +0200 Subject: [PATCH] [config] Remove case-insensitive queries and use libfmt for logging --- src/system/libs/seiscomp/config/config.cpp | 55 ++++++----- src/system/libs/seiscomp/config/config.h | 16 +-- src/system/libs/seiscomp/config/log.cpp | 7 -- src/system/libs/seiscomp/config/log.h | 8 +- .../libs/seiscomp/config/symboltable.cpp | 99 +------------------ src/system/libs/seiscomp/config/symboltable.h | 36 ++++--- 6 files changed, 58 insertions(+), 163 deletions(-) diff --git a/src/system/libs/seiscomp/config/config.cpp b/src/system/libs/seiscomp/config/config.cpp index 5c4b598c..b8112af8 100644 --- a/src/system/libs/seiscomp/config/config.cpp +++ b/src/system/libs/seiscomp/config/config.cpp @@ -278,15 +278,6 @@ Config::~Config() { -// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -void Config::setCaseSensitivityCheck(bool f) { - _symbolTable->setCaseSensitivityCheck(f); -} -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< - - - - // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bool Config::readConfig(const std::string &filename, int stage, bool raw) { _stage = stage; @@ -553,7 +544,7 @@ Config &Config::operator=(Config &&other) { _defaultNamespacePrefix = std::move(other._defaultNamespacePrefix); _logger = other._logger; other._logger = nullptr; - _symbolTable = other._symbolTable; _symbolTable = nullptr; + _symbolTable = other._symbolTable; other._symbolTable = nullptr; _trackVariables = std::move(other._trackVariables); _variables = std::move(other._variables); @@ -663,7 +654,7 @@ bool Config::parseFile(std::istream &is) { } if ( !_namespaces.empty() ) { - CONFIG_ERROR("Namespace '%s' has not been closed", _namespaces.back().c_str()); + CONFIG_ERROR("Namespace '%s' has not been closed", _namespaces.back()); return false; } @@ -698,7 +689,7 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { if ( tokens.size() < 2 ) { if ( (tokens.size() == 1) && (tokens[0] == BLOCK_END) ) { if ( _namespaces.empty() ) { - CONFIG_ERROR("Unexpected closing block: %s", entry.c_str()); + CONFIG_ERROR("Unexpected closing block: %s", entry); return false; } @@ -716,13 +707,13 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { return true; } else { - CONFIG_ERROR("Config entry malformed: Too few parameters for %s", entry.c_str()); + CONFIG_ERROR("Config entry malformed: Too few parameters for %s", entry); return false; } } if ( tokens[0][0] == '$' ) { - CONFIG_ERROR("Cannot assign to rvalue: %s", tokens[0].c_str()); + CONFIG_ERROR("Cannot assign to rvalue: %s", tokens[0]); return false; } @@ -731,18 +722,18 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { if ( tokens[0] == KEYWORD_INCLUDE ) { if ( tokens.size() > 2 ) { CONFIG_ERROR("Operator %s has too many operands -> %s file", - tokens[0].c_str(), tokens[0].c_str()); + tokens[0], tokens[0]); return false; } if ( !parseRValue(tokens[1], parsedValues, _symbolTable, _resolveReferences, false, &errmsg) ) { - CONFIG_ERROR("%s", errmsg.c_str()); + CONFIG_ERROR("%s", errmsg); return false; } if ( !handleInclude(parsedValues[0]) ) { - CONFIG_ERROR("Could not read include file %s", parsedValues[0].c_str()); + CONFIG_ERROR("Could not read include file %s", parsedValues[0]); return false; } } @@ -760,7 +751,7 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { parsedValues.clear(); if ( !parseRValue(tmp, parsedValues, _symbolTable, _resolveReferences, false, &errmsg) ) { - CONFIG_ERROR("%s", errmsg.c_str()); + CONFIG_ERROR("%s", errmsg); return false; } @@ -768,7 +759,7 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { for ( ; it != parsedValues.end(); ++it ) { if ( !_symbolTable->remove(_namespacePrefix + *it) ) { CONFIG_ERROR("Could not remove variable %s from symbol table", - (_namespacePrefix + (*it)).c_str()); + _namespacePrefix + (*it)); return false; } } @@ -780,7 +771,7 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { } else if ( tokens[1] == "=" ) { if ( tokens.size() < 3 ) { - CONFIG_ERROR("RValue missing in assignment: %s", entry.c_str()); + CONFIG_ERROR("RValue missing in assignment: %s", entry); return false; } @@ -790,14 +781,14 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { tmp += tokens[i]; if ( !eval(tmp, values, _resolveReferences, &errmsg) ) { - CONFIG_ERROR("%s", errmsg.c_str()); + CONFIG_ERROR("%s", errmsg); return false; } handleAssignment(tokens[0], tmp, values, comment); } else { - CONFIG_ERROR("Invalid entry: %s", entry.c_str()); + CONFIG_ERROR("Invalid entry: %s", entry); return false; } @@ -809,8 +800,10 @@ bool Config::handleEntry(const std::string& entry, const std::string& comment) { // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -bool Config::handleInclude(const std::string& fileName) { - if ( fileName.empty() ) return false; +bool Config::handleInclude(const std::string &fileName) { + if ( fileName.empty() ) { + return false; + } std::string tmpFileName(fileName); // Resolve ~ for home directory @@ -826,8 +819,13 @@ bool Config::handleInclude(const std::string& fileName) { // Change to the config file path to be able to handle relative paths if ( getcwd(oldPath, PATH_MAX) ) { std::string::size_type pos = _fileName.rfind("/"); - if ( pos != std::string::npos ) - chdir(_fileName.substr(0, pos).c_str()); + if ( pos != std::string::npos ) { + if ( chdir(_fileName.substr(0, pos).c_str()) ) { + CONFIG_ERROR("Cannot change into directory %s", + _fileName.substr(0, pos)); + return false; + } + } } } @@ -841,7 +839,10 @@ bool Config::handleInclude(const std::string& fileName) { } if ( isRelativePath ) { - chdir(oldPath); + if ( chdir(oldPath) ) { + CONFIG_ERROR("Cannot change back into directory %s", oldPath); + return false; + } } return true; diff --git a/src/system/libs/seiscomp/config/config.h b/src/system/libs/seiscomp/config/config.h index fec1ed12..2fb5a4e0 100644 --- a/src/system/libs/seiscomp/config/config.h +++ b/src/system/libs/seiscomp/config/config.h @@ -40,7 +40,7 @@ namespace Config { /** * Mapping of configuration variable to type */ -typedef std::map Variables; +using Variables = std::map; /** @@ -61,21 +61,13 @@ class SC_CONFIG_API Config { // Public interface // ------------------------------------------------------------------------ public: - /** When names are queried and this check is enabled, it will - * throw an exception if the same name is defined in a later stage - * with respect to case insensitive name comparison. - * This allows to check for parameter inconsistencies that are - * hard to track otherwise. - */ - void setCaseSensitivityCheck(bool); - /** Reads the given configuration file. * @param file name of the configuration files * @param stage Optional stage value to be set to each read symbol * @param raw Raw mode which does not resolv references like ${var} * @return true on success */ - bool readConfig(const std::string& file, int stage=-1, bool raw=false); + bool readConfig(const std::string &file, int stage=-1, bool raw=false); /** Writes the configuration to the given configuration file. * @param file name of the configuarion files @@ -327,7 +319,8 @@ class SC_CONFIG_API Config { // Private data members // ------------------------------------------------------------------------ private: - typedef std::deque Namespaces; + using Namespaces = std::deque; + int _stage; int _line; bool _resolveReferences; @@ -346,4 +339,5 @@ class SC_CONFIG_API Config { } // namespace Config } // namespace Seiscomp + #endif diff --git a/src/system/libs/seiscomp/config/log.cpp b/src/system/libs/seiscomp/config/log.cpp index becc8a4f..d4a6e88a 100644 --- a/src/system/libs/seiscomp/config/log.cpp +++ b/src/system/libs/seiscomp/config/log.cpp @@ -28,13 +28,6 @@ namespace Config { -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< -char log_msg_buffer[1024]; -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< - - - - // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Logger::~Logger() {} // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< diff --git a/src/system/libs/seiscomp/config/log.h b/src/system/libs/seiscomp/config/log.h index a84ecd20..b6cec7cd 100644 --- a/src/system/libs/seiscomp/config/log.h +++ b/src/system/libs/seiscomp/config/log.h @@ -23,6 +23,7 @@ #include +#include #include @@ -44,13 +45,10 @@ struct SC_CONFIG_API Logger { }; -extern char log_msg_buffer[1024]; - - #define CONFIG_LOG_CHANNEL(chan, msg, ...) \ if ( _logger ) {\ - snprintf(log_msg_buffer, 1023, msg, __VA_ARGS__);\ - _logger->log(chan, _fileName.c_str(), _line, log_msg_buffer);\ + auto line = fmt::sprintf(msg, __VA_ARGS__);\ + _logger->log(chan, _fileName.c_str(), _line, line.c_str());\ } diff --git a/src/system/libs/seiscomp/config/symboltable.cpp b/src/system/libs/seiscomp/config/symboltable.cpp index aa9ddbae..aef7393e 100644 --- a/src/system/libs/seiscomp/config/symboltable.cpp +++ b/src/system/libs/seiscomp/config/symboltable.cpp @@ -29,22 +29,6 @@ namespace Seiscomp { namespace Config { -namespace { - - -std::string toupper(const std::string &s) { - std::string tmp; - std::string::const_iterator it; - for ( it = s.begin(); it != s.end(); ++it ) - tmp += ::toupper(*it); - return tmp; -} - - -} - - - Symbol::Symbol(const std::string& name, const std::string &ns, const std::vector& values, const std::string& uri, @@ -118,20 +102,6 @@ std::string Symbol::toString() const { -// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -SymbolTable::SymbolTable() : _csCheck(false), _objectCount(0), _logger(NULL) {} -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< - - - -// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -void SymbolTable::setCaseSensitivityCheck(bool f) { - _csCheck = f; -} -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< - - - // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> void SymbolTable::setLogger(Logger *l) { _logger = l; @@ -171,9 +141,6 @@ void SymbolTable::add(const std::string& name, // Update the last line in the parsed content itp.first->second.line = line; - - // Register mapping to case-sensitive name - _cisymbols[toupper(name)] = itp.first; } // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< @@ -182,8 +149,7 @@ void SymbolTable::add(const std::string& name, // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> void SymbolTable::add(const Symbol &symbol) { - std::pair itp; - itp = _symbols.insert(Symbols::value_type(symbol.name, Symbol())); + auto itp = _symbols.insert(Symbols::value_type(symbol.name, Symbol())); if ( itp.second ) { Symbol &newSymbol = itp.first->second; @@ -193,9 +159,6 @@ void SymbolTable::add(const Symbol &symbol) { else { itp.first->second = symbol; } - - // Register mapping to case-sensitive name - _cisymbols[toupper(symbol.name)] = itp.first; } // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< @@ -204,13 +167,7 @@ void SymbolTable::add(const Symbol &symbol) { // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> bool SymbolTable::remove(const std::string& name) { - CISymbols::iterator ci_it; - ci_it = _cisymbols.find(toupper(name)); - if ( ci_it != _cisymbols.end() ) - _cisymbols.erase(ci_it); - - Symbols::iterator it; - it = _symbols.find(name); + auto it = _symbols.find(name); if ( it != _symbols.end() ) { SymbolOrder::iterator ito = std::find(_symbolOrder.begin(), _symbolOrder.end(), &it->second); if ( ito != _symbolOrder.end() ) @@ -228,17 +185,12 @@ bool SymbolTable::remove(const std::string& name) { // >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Symbol* SymbolTable::get(const std::string& name) { - Symbols::iterator it = _symbols.find(name); + auto it = _symbols.find(name); if ( it != _symbols.end() ) { - if ( _csCheck && checkCI(name, &it->second) ) - return NULL; - return &it->second; } - if ( _csCheck ) checkCI(name, NULL); - - return NULL; + return nullptr; } // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< @@ -249,51 +201,10 @@ Symbol* SymbolTable::get(const std::string& name) { const Symbol* SymbolTable::get(const std::string& name) const { Symbols::const_iterator it = _symbols.find(name); if ( it != _symbols.end() ) { - if ( _csCheck && checkCI(name, &it->second) ) - return NULL; - return &it->second; } - if ( _csCheck ) checkCI(name, NULL); - - return NULL; -} -// <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< - - - - -// >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> -bool SymbolTable::checkCI(const std::string &name, const Symbol *s) const { - CISymbols::const_iterator it = _cisymbols.find(toupper(name)); - // No upper case entry found - if ( it == _cisymbols.end() ) return false; - - if ( s == NULL ) { - int _line = it->second->second.line; - const std::string &_fileName = it->second->second.uri; - - // Issue warning - CONFIG_WARNING("%s should define %s which is not defined itself: names are case-sensitive!", - it->second->second.name.c_str(), - name.c_str()); - return true; - } - - const Symbol *cisym = &it->second->second; - // Defined at a later stage? - if ( s->stage >= 0 && (cisym->stage > s->stage || cisym->line > s->line) ) { - int _line = cisym->line; - const std::string &_fileName = cisym->uri; - - // Issue warning - CONFIG_WARNING("%s should override %s but does not: names are case-sensitive!", - cisym->name.c_str(), name.c_str()); - return true; - } - - return false; + return nullptr; } // <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< diff --git a/src/system/libs/seiscomp/config/symboltable.h b/src/system/libs/seiscomp/config/symboltable.h index 9cded4fb..d809624e 100644 --- a/src/system/libs/seiscomp/config/symboltable.h +++ b/src/system/libs/seiscomp/config/symboltable.h @@ -17,9 +17,11 @@ * gempa GmbH. * ***************************************************************************/ + #ifndef __SEISCOMP_CONFIG_SYMBOLTABLE__ #define __SEISCOMP_CONFIG_SYMBOLTABLE__ + #include #include #include @@ -28,6 +30,7 @@ #include + namespace Seiscomp { namespace Config { @@ -65,23 +68,22 @@ struct SC_CONFIG_API Symbol { class SC_CONFIG_API SymbolTable { - private: - typedef std::map Symbols; - typedef std::vector SymbolOrder; - typedef std::map CISymbols; + using Symbols = std::map; + using SymbolOrder = std::vector; + public: - typedef SymbolOrder::const_iterator iterator; - typedef std::set IncludedFiles; - typedef IncludedFiles::iterator file_iterator; + using iterator = SymbolOrder::const_iterator; + using IncludedFiles = std::set; + using file_iterator = IncludedFiles::iterator; + public: - SymbolTable(); + SymbolTable() = default; public: - void setCaseSensitivityCheck(bool); void setLogger(Logger *); Logger *logger(); @@ -114,22 +116,18 @@ class SC_CONFIG_API SymbolTable { iterator begin(); iterator end(); - private: - //! Returns true if an inconsistent definition has been found - bool checkCI(const std::string &name, const Symbol *) const; private: - bool _csCheck; - Symbols _symbols; - CISymbols _cisymbols; - SymbolOrder _symbolOrder; - IncludedFiles _includedFiles; - int _objectCount; - Logger *_logger; + Symbols _symbols; + SymbolOrder _symbolOrder; + IncludedFiles _includedFiles; + int _objectCount{0}; + Logger *_logger{nullptr}; }; } // namespace Config } // namespace Seiscomp + #endif