Simplified lifetime management of tests

- Addresses Sonarcloud issues: - Rewrite the code so that you no longer need this "delete". - Make the type of this variable a reference-to-const.
owasp-modsecurity · Oct 7, 2024 · ba98278 · ba98278
1 parent a1ba1bd
commit ba98278
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 34 deletions.
diff --git a/test/common/modsecurity_test.cc b/test/common/modsecurity_test.cc
@@ -72,18 +72,11 @@ bool ModSecurityTest<T>::load_test_json(const std::string &file) {
     for ( int i = 0; i < num_tests; i++ ) {
         yajl_val obj = node->u.array.values[i];
 
-        T *u = T::from_yajl_node(obj);
+        auto u = std::unique_ptr<T>(T::from_yajl_node(obj));
         u->filename = file;
 
-        if (this->count(u->filename + ":" + u->name) == 0) {
-            auto vec = new std::vector<T *>;
-            vec->push_back(u);
-            std::string filename(u->filename + ":" + u->name);
-            this->insert({filename, vec});
-        } else {
-            auto vec = this->at(u->filename + ":" + u->name);
-            vec->push_back(u);
-        }
+        const auto key = u->filename + ":" + u->name;
+        (*this)[key].push_back(std::move(u));
     }
 
     yajl_tree_free(node);

diff --git a/test/common/modsecurity_test.h b/test/common/modsecurity_test.h
@@ -29,7 +29,7 @@ extern std::string default_test_path;
 namespace modsecurity_test {
 
 template <class T> class ModSecurityTest :
-    public std::unordered_map<std::string, std::vector<T *> *> {
+    public std::unordered_map<std::string, std::vector<std::unique_ptr<T>>> {
  public:
     ModSecurityTest()
         : m_test_number(0),

diff --git a/test/regression/regression.cc b/test/regression/regression.cc
@@ -111,13 +111,14 @@ void actions(ModSecurityTestResults<RegressionTest> *r,
 }
 
 void perform_unit_test(ModSecurityTest<RegressionTest> *test,
-    std::vector<RegressionTest *> *tests,
-    ModSecurityTestResults<RegressionTestResult> *res, int *count) {
-    for (RegressionTest *t : *tests) {
+    std::vector<std::unique_ptr<RegressionTest>> &tests,
+    ModSecurityTestResults<RegressionTestResult> *res, int *count)
+{
+    for (auto &t : tests) {
         ModSecurityTestResults<RegressionTest> r;
         RegressionTestResult *testRes = new RegressionTestResult();
 
-        testRes->test = t;
+        testRes->test = t.get();
         r.status = 200;
         (*count)++;
 
@@ -468,9 +469,9 @@ int main(int argc, char **argv)
     ModSecurityTestResults<RegressionTestResult> res;
     for (const std::string &a : keyList) {
         test_number++;
-        if ((test.m_test_number == 0) 
+        if ((test.m_test_number == 0)
             || (test_number == test.m_test_number)) {
-            std::vector<RegressionTest *> *tests = test[a];
+            auto &tests = test[a];
             perform_unit_test(&test, tests, &res, &counter);
         }
     }
@@ -523,14 +524,6 @@ int main(int argc, char **argv)
         std::cout << "disabled test(s)." << RESET << std::endl;
     }
 
-    for (auto a : test) {
-        std::vector<RegressionTest *> *vec = a.second;
-        for (int i = 0; i < vec->size(); i++) {
-            delete vec->at(i);
-        }
-        delete vec;
-    }
-
     return failed;
 #endif
 }
diff --git a/test/unit/unit.cc b/test/unit/unit.cc
@@ -250,8 +250,8 @@ int main(int argc, char **argv) {
     }
 
     for (auto& [filename, tests] : test) {
-        total += tests->size();
-        for (auto t : *tests) {
+        total += tests.size();
+        for (auto &t : tests) {
             ModSecurityTestResults<UnitTest> r;
 
             if (!test.m_automake_output) {
@@ -311,12 +311,5 @@ int main(int argc, char **argv) {
         }
     }
 
-    for (auto a : test) {
-        auto vec = a.second;
-        for(auto t : *vec)
-            delete t;
-        delete vec;
-    }
-
     return failed;
 }