From a1ba1bd275f2d8f4e7ea4eb3f2da87966fc1e030 Mon Sep 17 00:00:00 2001 From: Eduardo Arias Date: Tue, 10 Sep 2024 14:47:00 -0300 Subject: [PATCH] Minor codebase improvements suggested by Sonarcloud - src/modsecurity.cc - Replace the redundant type with "auto". - src/transaction.cc - Avoid this unnecessary copy by using a "const" reference. - test/common/custom_debug_log.cc - Use "=default" instead of the default implementation of this special member functions. - Removed the unnecessary destructor override instead. - Annotate this function with "override" or "final". - Removed the unnecessary destructor override instead. - Remove this "const" qualifier from the return type in all declarations. - test/common/modsecurity_test_context.h - Replace the redundant type with "auto". - test/regression/regression.cc - Use the "nullptr" literal. - Replace this declaration by a structured binding declaration. - Replace "reinterpret_cast" with a safer operation. --- src/modsecurity.cc | 4 ++-- src/transaction.cc | 2 +- test/common/custom_debug_log.cc | 6 ++---- test/common/custom_debug_log.h | 5 ++--- test/common/modsecurity_test_context.h | 4 ++-- test/regression/regression.cc | 26 +++++++++++++------------- 6 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/modsecurity.cc b/src/modsecurity.cc index e1b29857e1..8f943b7f76 100644 --- a/src/modsecurity.cc +++ b/src/modsecurity.cc @@ -199,13 +199,13 @@ void ModSecurity::serverLog(void *data, const RuleMessage &rm) { if (m_logProperties & TextLogProperty) { auto d = rm.log(); - const void *a = static_cast(d.c_str()); + auto a = static_cast(d.c_str()); m_logCb(data, a); return; } if (m_logProperties & RuleMessageLogProperty) { - const void *a = static_cast(&rm); + auto a = static_cast(&rm); m_logCb(data, a); return; } diff --git a/src/transaction.cc b/src/transaction.cc index 1b055ad084..1892252310 100644 --- a/src/transaction.cc +++ b/src/transaction.cc @@ -1529,7 +1529,7 @@ std::string Transaction::toOldAuditLogFormat(int parts, } if (parts & audit_log::AuditLog::HAuditLogPart) { audit_log << "--" << trailer << "-" << "H--" << std::endl; - for (auto a : m_rulesMessages) { + for (const auto &a : m_rulesMessages) { audit_log << a.log(0, m_httpCodeReturned) << std::endl; } audit_log << std::endl; diff --git a/test/common/custom_debug_log.cc b/test/common/custom_debug_log.cc index 7f69b58c1c..fbf97ab9dc 100644 --- a/test/common/custom_debug_log.cc +++ b/test/common/custom_debug_log.cc @@ -23,8 +23,6 @@ namespace modsecurity_test { - CustomDebugLog::~CustomDebugLog() {} - void CustomDebugLog::write(int level, const std::string &message) { m_log << "[" << level << "] " << message << std::endl; } @@ -36,13 +34,13 @@ namespace modsecurity_test { m_log << msgf << std::endl; } - bool const CustomDebugLog::contains(const std::string &pattern) const { + bool CustomDebugLog::contains(const std::string &pattern) const { modsecurity::Utils::Regex re(pattern); std::string s = m_log.str(); return modsecurity::Utils::regex_search(s, re); } - std::string const CustomDebugLog::log_messages() const { + std::string CustomDebugLog::log_messages() const { return m_log.str(); } diff --git a/test/common/custom_debug_log.h b/test/common/custom_debug_log.h index f1868acd6e..92857d1006 100644 --- a/test/common/custom_debug_log.h +++ b/test/common/custom_debug_log.h @@ -26,13 +26,12 @@ namespace modsecurity_test { class CustomDebugLog : public modsecurity::debug_log::DebugLog { public: CustomDebugLog *new_instance(); - ~CustomDebugLog(); void write(int level, const std::string& message) override; void write(int level, const std::string &id, const std::string &uri, const std::string &msg) override; - bool const contains(const std::string& pattern) const; - std::string const log_messages() const; + bool contains(const std::string& pattern) const; + std::string log_messages() const; std::string error_log_messages(); int getDebugLogLevel() override; diff --git a/test/common/modsecurity_test_context.h b/test/common/modsecurity_test_context.h index 369717ec19..7765443b21 100644 --- a/test/common/modsecurity_test_context.h +++ b/test/common/modsecurity_test_context.h @@ -36,8 +36,8 @@ namespace modsecurity_test private: static void logCb(void *data, const void *msgv) { - const char *msg = reinterpret_cast(msgv); - std::stringstream *ss = (std::stringstream *)data; + auto msg = reinterpret_cast(msgv); + auto ss = reinterpret_cast(data); *ss << msg << std::endl; } }; diff --git a/test/regression/regression.cc b/test/regression/regression.cc index 5992d2cf47..62ab1e8919 100644 --- a/test/regression/regression.cc +++ b/test/regression/regression.cc @@ -97,15 +97,15 @@ void actions(ModSecurityTestResults *r, if (it.status != 0) { r->status = it.status; } - if (it.url != NULL) { + if (it.url != nullptr) { r->location.append(it.url); free(it.url); - it.url = NULL; + it.url = nullptr; } - if (it.log != NULL) { + if (it.log != nullptr) { *serverLog << it.log; free(it.log); - it.log = NULL; + it.log = nullptr; } } } @@ -283,9 +283,9 @@ void perform_unit_test(ModSecurityTest *test, actions(&r, &modsec_transaction, &context.m_server_log); - for (const auto &headers : t->request_headers) { - modsec_transaction.addRequestHeader(headers.first.c_str(), - headers.second.c_str()); + for (const auto &[name, value] : t->request_headers) { + modsec_transaction.addRequestHeader(name.c_str(), + value.c_str()); } modsec_transaction.processRequestHeaders(); @@ -297,9 +297,9 @@ void perform_unit_test(ModSecurityTest *test, modsec_transaction.processRequestBody(); actions(&r, &modsec_transaction, &context.m_server_log); - for (const auto &headers : t->response_headers) { - modsec_transaction.addResponseHeader(headers.first.c_str(), - headers.second.c_str()); + for (const auto &[name, value] : t->response_headers) { + modsec_transaction.addResponseHeader(name.c_str(), + value.c_str()); } modsec_transaction.processResponseHeaders(r.status, @@ -314,7 +314,7 @@ void perform_unit_test(ModSecurityTest *test, modsec_transaction.processLogging(); - const auto *d = reinterpret_cast(context.m_modsec_rules.m_debugLog); + const auto *d = static_cast(context.m_modsec_rules.m_debugLog); if (!d->contains(t->debug_log)) { if (test->m_automake_output) { @@ -455,8 +455,8 @@ int main(int argc, char **argv) int counter = 0; std::list keyList; - for (const auto &a : test) { - keyList.push_back(a.first); + for (const auto &[name, tests] : test) { + keyList.push_back(name); } keyList.sort();