From d5fe21ce3c88a2e7339f1a7620e3b540a331edc6 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 12 Aug 2015 15:22:00 -0300 Subject: [PATCH] Code cosmetics: reduce the amount of cppcheck warnings --- headers/modsecurity/assay.h | 8 ++++---- headers/modsecurity/rules.h | 2 ++ headers/modsecurity/rules_properties.h | 14 +++++++++----- src/actions/set_var.cc | 2 +- src/actions/transformations/css_decode.cc | 2 +- .../transformations/html_entity_decode.cc | 2 +- src/actions/transformations/js_decode.cc | 2 +- .../transformations/normalise_path_win.cc | 2 +- src/actions/transformations/url_decode_uni.cc | 2 +- src/assay.cc | 16 +++++++--------- src/audit_log_writer_parallel.cc | 1 - src/operators/validate_url_encoding.cc | 2 +- src/operators/verify_cc.cc | 1 - src/parser/seclang-parser.yy | 10 +++++----- src/request_body_processor/multipart_blob.cc | 2 +- src/rules.cc | 18 +++++++----------- src/unique_id.cc | 2 +- src/utils.cc | 2 +- src/utils/ip_tree.cc | 3 +-- src/utils/md5.cc | 2 +- src/utils/md5.h | 2 +- src/utils/regex.cc | 1 - test/common/modsecurity_test.cc | 4 +--- test/unit/unit.cc | 3 +-- 24 files changed, 49 insertions(+), 56 deletions(-) diff --git a/headers/modsecurity/assay.h b/headers/modsecurity/assay.h index a5eec221..c93b7af2 100644 --- a/headers/modsecurity/assay.h +++ b/headers/modsecurity/assay.h @@ -227,10 +227,10 @@ class Assay { int getResponseBodyLenth(); std::list> - resolve_variable(std::string var); - std::string* resolve_variable_first(std::string); - std::string* resolve_variable_first(const std::string collectionName, - std::string var); + resolve_variable(const std::string& var); + std::string* resolve_variable_first(const std::string& key); + std::string* resolve_variable_first(const std::string& collectionName, + const std::string& var); void store_variable(std::string, std::string value); bool update_variable_first(std::string var, const std::string &value); diff --git a/headers/modsecurity/rules.h b/headers/modsecurity/rules.h index 1aca0350..7cc5e04b 100644 --- a/headers/modsecurity/rules.h +++ b/headers/modsecurity/rules.h @@ -47,6 +47,7 @@ class Rules : public RulesProperties { Rules() : debugLog(NULL), RulesProperties(NULL), + m_referenceCount(0), unicode_codepage(0) { unicode_map_table = reinterpret_cast( malloc(sizeof(int)*65536)); @@ -55,6 +56,7 @@ class Rules : public RulesProperties { explicit Rules(DebugLog *customLog) : debugLog(NULL), + m_referenceCount(0), unicode_codepage(0), RulesProperties(customLog) { unicode_map_table = reinterpret_cast( diff --git a/headers/modsecurity/rules_properties.h b/headers/modsecurity/rules_properties.h index bb1a46f2..ba438b66 100644 --- a/headers/modsecurity/rules_properties.h +++ b/headers/modsecurity/rules_properties.h @@ -45,8 +45,11 @@ class RulesProperties { customDebugLog(NULL), remoteRulesActionOnFailed(AbortOnFailedRemoteRulesAction), requestBodyLimit(0), + secRequestBodyAccess(false), + secResponseBodyAccess(false), requestBodyLimitAction(ProcessPartialBodyLimitAction), responseBodyLimit(0), + debugLevel(0), responseBodyLimitAction(ProcessPartialBodyLimitAction), secRuleEngine(DetectionOnlyRuleEngine) { } @@ -54,6 +57,9 @@ class RulesProperties { : audit_log(NULL), customDebugLog(customDebugLog), remoteRulesActionOnFailed(AbortOnFailedRemoteRulesAction), + secRequestBodyAccess(false), + secResponseBodyAccess(false), + debugLevel(0), requestBodyLimit(0), requestBodyLimitAction(ProcessPartialBodyLimitAction), responseBodyLimit(0), @@ -157,14 +163,12 @@ class RulesProperties { DebugLog *customDebugLog; - int sec_audit_type; - bool sec_audit_engine; - bool sec_request_body_access; - bool sec_response_body_access; + bool secRequestBodyAccess; + bool secResponseBodyAccess; std::string audit_log_path; std::string audit_log_parts; std::string debug_log_path; - int debug_level; + int debugLevel; std::list components; diff --git a/src/actions/set_var.cc b/src/actions/set_var.cc index dc032a19..800e2de0 100644 --- a/src/actions/set_var.cc +++ b/src/actions/set_var.cc @@ -31,7 +31,7 @@ SetVar::SetVar(std::string action) bool SetVar::init(std::string *error) { - size_t pos = std::string::npos; + size_t pos; // Resolv operation operation = setToOne; diff --git a/src/actions/transformations/css_decode.cc b/src/actions/transformations/css_decode.cc index ca9c978f..ceb5a5e7 100644 --- a/src/actions/transformations/css_decode.cc +++ b/src/actions/transformations/css_decode.cc @@ -37,7 +37,7 @@ namespace transformations { std::string CssDecode::evaluate(std::string value, Assay *assay) { char *tmp = strdup(value.c_str()); - int res = css_decode_inplace((unsigned char *)tmp, value.size()); + css_decode_inplace((unsigned char *)tmp, value.size()); std::string ret(tmp, 0, value.size()); free(tmp); return ret; diff --git a/src/actions/transformations/html_entity_decode.cc b/src/actions/transformations/html_entity_decode.cc index be5c87cb..0274d80f 100644 --- a/src/actions/transformations/html_entity_decode.cc +++ b/src/actions/transformations/html_entity_decode.cc @@ -37,7 +37,7 @@ namespace transformations { std::string HtmlEntityDecode::evaluate(std::string value, Assay *assay) { char *tmp = strdup(value.c_str()); - int res = html_entities_decode_inplace((unsigned char *)tmp, value.size()); + html_entities_decode_inplace((unsigned char *)tmp, value.size()); std::string ret(""); ret.assign(tmp); free(tmp); diff --git a/src/actions/transformations/js_decode.cc b/src/actions/transformations/js_decode.cc index a326d039..16d97c06 100644 --- a/src/actions/transformations/js_decode.cc +++ b/src/actions/transformations/js_decode.cc @@ -37,7 +37,7 @@ namespace transformations { std::string JsDecode::evaluate(std::string value, Assay *assay) { char *tmp = strdup(value.c_str()); - int res = js_decode_nonstrict_inplace((unsigned char *)tmp, value.size()); + js_decode_nonstrict_inplace((unsigned char *)tmp, value.size()); std::string ret(""); ret.assign(tmp); free(tmp); diff --git a/src/actions/transformations/normalise_path_win.cc b/src/actions/transformations/normalise_path_win.cc index 268a0d8f..d9e135ac 100644 --- a/src/actions/transformations/normalise_path_win.cc +++ b/src/actions/transformations/normalise_path_win.cc @@ -38,7 +38,7 @@ std::string NormalisePathWin::evaluate(std::string value, Assay *assay) { int changed; char *tmp = strdup(value.c_str()); - int res = normalize_path_inplace((unsigned char *)tmp, + normalize_path_inplace((unsigned char *)tmp, value.size(), 1, &changed); std::string ret(""); ret.assign(tmp); diff --git a/src/actions/transformations/url_decode_uni.cc b/src/actions/transformations/url_decode_uni.cc index d315ae7d..7139de6a 100644 --- a/src/actions/transformations/url_decode_uni.cc +++ b/src/actions/transformations/url_decode_uni.cc @@ -38,7 +38,7 @@ std::string UrlDecodeUni::evaluate(std::string value, Assay *assay) { int changed = 0; char *tmp = strdup(value.c_str()); - int res = urldecode_uni_nonstrict_inplace_ex(assay, (unsigned char *)tmp, + urldecode_uni_nonstrict_inplace_ex(assay, (unsigned char *)tmp, value.size(), &changed); std::string ret(""); ret.assign(tmp); diff --git a/src/assay.cc b/src/assay.cc index c2a81dc8..228a1f0a 100644 --- a/src/assay.cc +++ b/src/assay.cc @@ -579,7 +579,6 @@ int Assay::processRequestBody() { * */ char sep1 = '&'; - const char *pos = strchr(content.c_str(), '?'); std::vector key_value = split(content.c_str(), sep1); @@ -741,7 +740,6 @@ int Assay::processResponseHeaders() { */ int Assay::addResponseHeader(const std::string& key, const std::string& value) { - std::string *names = resolve_variable_first("RESPONSE_HEADERS_NAMES"); m_responseHeadersNames->assign(*m_responseHeadersNames + " " + key); this->store_variable("RESPONSE_HEADERS:" + key, value); @@ -1324,7 +1322,7 @@ void Assay::delete_variable(std::string key) { std::list> - Assay::resolve_variable(std::string var) { + Assay::resolve_variable(const std::string& var) { std::list> l; std::pair pair; @@ -1335,7 +1333,7 @@ std::list> l.push_back(pair); } - if (l.size() == 0) { + if (l.empty()) { for (auto &x : m_variables_strings) { if ((x.first.substr(0, var.size() + 1).compare(var + ":") != 0) && (x.first != var)) { @@ -1359,7 +1357,7 @@ std::list> l.push_back(pair); } - if (l.size() == 0) { + if (l.empty()) { for (auto &x : *a.second) { if ((x.first.substr(0, var.size() + 1).compare(var + ":") != 0) && (x.first != var)) { @@ -1383,7 +1381,7 @@ void Assay::serverLog(const std::string& msg) { std::cerr << "Server log is not ready : " << msg << std::endl; } -std::string* Assay::resolve_variable_first(std::string var) { +std::string* Assay::resolve_variable_first(const std::string& var) { auto range = m_variables_strings.equal_range(var); for (auto it = range.first; it != range.second; ++it) { @@ -1400,8 +1398,8 @@ std::string* Assay::resolve_variable_first(std::string var) { } -std::string* Assay::resolve_variable_first(const std::string collectionName, - std::string var) { +std::string* Assay::resolve_variable_first(const std::string& collectionName, + const std::string& var) { for (auto &a : collections) { if (tolower(a.first) == tolower(collectionName)) { auto range = a.second->equal_range(toupper(collectionName) @@ -1418,9 +1416,9 @@ std::string* Assay::resolve_variable_first(const std::string collectionName, void Assay::setCollection(const std::string& collectionName, const std::string& variableName, const std::string& targetValue) { - ModSecurityStringVariables *collection; try { + ModSecurityStringVariables *collection; collection = collections.at(toupper(collectionName)); collection->storeOrUpdateVariable(toupper(collectionName) + ":" + variableName, targetValue); diff --git a/src/audit_log_writer_parallel.cc b/src/audit_log_writer_parallel.cc index c89a8d46..b0b796b5 100644 --- a/src/audit_log_writer_parallel.cc +++ b/src/audit_log_writer_parallel.cc @@ -47,7 +47,6 @@ inline std::string AuditLogWriterParallel::logFilePath(time_t *t, int part) { struct tm timeinfo; char tstr[300]; - size_t len; std::string name(""); localtime_r(t, &timeinfo); diff --git a/src/operators/validate_url_encoding.cc b/src/operators/validate_url_encoding.cc index d1f111dc..cbb0b458 100644 --- a/src/operators/validate_url_encoding.cc +++ b/src/operators/validate_url_encoding.cc @@ -27,7 +27,7 @@ int ValidateUrlEncoding::validate_url_encoding(const char *input, uint64_t input_length) { int i; - if ((input == NULL) || (input_length < 0)) { + if ((input == NULL) || (input_length <= 0)) { return -1; } diff --git a/src/operators/verify_cc.cc b/src/operators/verify_cc.cc index 75b080ca..3c45527c 100644 --- a/src/operators/verify_cc.cc +++ b/src/operators/verify_cc.cc @@ -70,7 +70,6 @@ bool VerifyCC::evaluate(Assay *assay, const std::string &i) { int offset = 0; bool is_cc = false; int target_length = i.length(); - const char *target = i.c_str(); for (offset = 0; offset < target_length; offset++) { std::string shiftedString(i, offset, i.length() - offset); diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy index 73afe13a..89389df2 100644 --- a/src/parser/seclang-parser.yy +++ b/src/parser/seclang-parser.yy @@ -311,19 +311,19 @@ expression: } | CONFIG_DIR_REQ_BODY SPACE CONFIG_VALUE_ON { - driver.sec_request_body_access = true; + driver.secRequestBodyAccess = true; } | CONFIG_DIR_REQ_BODY SPACE CONFIG_VALUE_OFF { - driver.sec_request_body_access = false; + driver.secRequestBodyAccess = false; } | CONFIG_DIR_RES_BODY SPACE CONFIG_VALUE_ON { - driver.sec_request_body_access = true; + driver.secResponseBodyAccess = true; } | CONFIG_DIR_RES_BODY SPACE CONFIG_VALUE_OFF { - driver.sec_request_body_access = false; + driver.secResponseBodyAccess = false; } | CONFIG_COMPONENT_SIG { @@ -332,7 +332,7 @@ expression: /* Debug log: start */ | CONFIG_DIR_DEBUG_LVL { - driver.debug_level = atoi($1.c_str()); + driver.debugLevel = atoi($1.c_str()); } | CONFIG_DIR_DEBUG_LOG { diff --git a/src/request_body_processor/multipart_blob.cc b/src/request_body_processor/multipart_blob.cc index 5070d72a..ad81b455 100644 --- a/src/request_body_processor/multipart_blob.cc +++ b/src/request_body_processor/multipart_blob.cc @@ -56,7 +56,7 @@ bool MultipartBlob::processContent() { } bool contentTypeLine = processContentTypeLine(secondLine); - if (dispositionLine == false) { + if (contentTypeLine == false) { return false; } diff --git a/src/rules.cc b/src/rules.cc index 173cd81f..4f840cf5 100644 --- a/src/rules.cc +++ b/src/rules.cc @@ -188,12 +188,10 @@ int Rules::merge(Driver *from) { } this->secRuleEngine = from->secRuleEngine; - this->sec_audit_type = from->sec_audit_type; - this->sec_audit_engine = from->sec_audit_engine; - this->sec_request_body_access = from->sec_request_body_access; - this->sec_response_body_access = from->sec_response_body_access; + this->secRequestBodyAccess = from->secRequestBodyAccess; + this->secResponseBodyAccess = from->secResponseBodyAccess; this->debug_log_path = from->debug_log_path; - this->debug_level = from->debug_level; + this->debugLevel = from->debugLevel; this->components = from->components; this->requestBodyLimit = from->requestBodyLimit; this->responseBodyLimit = from->responseBodyLimit; @@ -210,7 +208,7 @@ int Rules::merge(Driver *from) { this->audit_log = from->audit_log; this->audit_log->refCountIncrease(); - this->debugLog->setDebugLevel(this->debug_level); + this->debugLog->setDebugLevel(this->debugLevel); this->debugLog->setOutputFile(this->debug_log_path); return 0; @@ -228,10 +226,8 @@ int Rules::merge(Rules *from) { } this->secRuleEngine = from->secRuleEngine; - this->sec_audit_type = from->sec_audit_type; - this->sec_audit_engine = from->sec_audit_engine; - this->sec_request_body_access = from->sec_request_body_access; - this->sec_response_body_access = from->sec_response_body_access; + this->secRequestBodyAccess = from->secRequestBodyAccess; + this->secResponseBodyAccess = from->secResponseBodyAccess; this->components = from->components; this->requestBodyLimit = from->requestBodyLimit; this->responseBodyLimit = from->responseBodyLimit; @@ -248,7 +244,7 @@ int Rules::merge(Rules *from) { this->audit_log = from->audit_log; this->audit_log->refCountIncrease(); - this->debugLog->setDebugLevel(this->debug_level); + this->debugLog->setDebugLevel(this->debugLevel); this->debugLog->setOutputFile(this->debug_log_path); return 0; diff --git a/src/unique_id.cc b/src/unique_id.cc index 508e7f74..45ebec7d 100644 --- a/src/unique_id.cc +++ b/src/unique_id.cc @@ -190,7 +190,7 @@ std::string UniqueId::ethernetMacAddress() { if (GetAdaptersInfo(pAdapterInfo, &ulOutBufLen) == ERROR_BUFFER_OVERFLOW) { free(pAdapterInfo); - pAdapterInfo = reinterpret_castmalloc(ulOutBufLen)); + pAdapterInfo = reinterpret_cast(malloc(ulOutBufLen)); if (!pAdapterInfo) { goto failed; } diff --git a/src/utils.cc b/src/utils.cc index 84e7a194..03ae6e7b 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -73,7 +73,7 @@ double random_number(const double from, const double to) { std::string dash_if_empty(const std::string& str) { - if (&str == NULL || str.empty()) { + if (str.empty()) { return "-"; } diff --git a/src/utils/ip_tree.cc b/src/utils/ip_tree.cc index 9dc79b1d..7cec0925 100644 --- a/src/utils/ip_tree.cc +++ b/src/utils/ip_tree.cc @@ -80,10 +80,9 @@ IpTree::~IpTree() { bool IpTree::addFromBuffer(std::istream *ss, std::string *error) { char *error_msg = NULL; - int res = 0; for (std::string line; std::getline(*ss, line); ) { - res = ip_tree_from_param(line.c_str(), &m_tree, &error_msg); + int res = ip_tree_from_param(line.c_str(), &m_tree, &error_msg); if (res != 0) { if (error_msg != NULL) { error->assign(error_msg); diff --git a/src/utils/md5.cc b/src/utils/md5.cc index e7babf76..fdebb068 100644 --- a/src/utils/md5.cc +++ b/src/utils/md5.cc @@ -354,7 +354,7 @@ std::ostream& operator<<(std::ostream& out, MD5 md5) ////////////////////////////// -std::string md5(const std::string str) +std::string md5(const std::string& str) { MD5 md5 = MD5(str); diff --git a/src/utils/md5.h b/src/utils/md5.h index 443895db..66b63f08 100644 --- a/src/utils/md5.h +++ b/src/utils/md5.h @@ -88,6 +88,6 @@ private: static inline void II(uint4 &a, uint4 b, uint4 c, uint4 d, uint4 x, uint4 s, uint4 ac); }; -std::string md5(const std::string str); +std::string md5(const std::string& str); #endif \ No newline at end of file diff --git a/src/utils/regex.cc b/src/utils/regex.cc index a48d6a2d..f0cf1940 100644 --- a/src/utils/regex.cc +++ b/src/utils/regex.cc @@ -53,7 +53,6 @@ int regex_search(const std::string& s, SMatch *match, } int regex_search(const std::string& s, Regex regex) { - std::string match; pcrecpp::RE re(regex.pattern); return re.PartialMatch(s); } diff --git a/test/common/modsecurity_test.cc b/test/common/modsecurity_test.cc index 4661f52e..c6a353c2 100644 --- a/test/common/modsecurity_test.cc +++ b/test/common/modsecurity_test.cc @@ -45,7 +45,6 @@ std::string ModSecurityTest::header() { template bool ModSecurityTest::load_test_json(std::string file) { - std::vector tests; char errbuf[1024]; yajl_val node; @@ -128,9 +127,8 @@ std::pair>* ModSecurityTest::load_tests() { template void ModSecurityTest::cmd_options(int argc, char **argv) { - int option_char; - #if HAS_GETOPT + int option_char; GetOpt getopt(argc, argv, "hvct:"); while ((option_char = getopt()) != EOF) { diff --git a/test/unit/unit.cc b/test/unit/unit.cc index 2c97b8ba..b38a7a89 100644 --- a/test/unit/unit.cc +++ b/test/unit/unit.cc @@ -58,13 +58,12 @@ void print_help() { void perform_unit_test(UnitTest *t, ModSecurityTestResults* res) { const char *error = NULL; - int ret = 0; if (t->type == "op") { Operator *op = Operator::instantiate("\"@" + t->name + \ " " + t->param + "\""); op->init(&error); - ret = op->evaluate(NULL, t->input); + int ret = op->evaluate(NULL, t->input); if (ret != t->ret) { t->obtained = ret; res->push_back(t);