Pass std::unique_ptr<Operator> to RuleWithOperator's constructor to m…

…ake transfer of ownership explicit
eduar-hte · Aug 22, 2024 · 3183193 · 3183193
1 parent 0674afb
commit 3183193
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 30 deletions.
diff --git a/headers/modsecurity/rule_with_operator.h b/headers/modsecurity/rule_with_operator.h
@@ -14,12 +14,9 @@
  */
 
 #ifdef __cplusplus
-#include <stack>
 #include <vector>
 #include <string>
-#include <list>
 #include <memory>
-#include <utility>
 #endif
 
 #ifndef HEADERS_MODSECURITY_RULE_WITH_OPERATOR_H_
@@ -38,7 +35,7 @@ namespace modsecurity {
 
 class RuleWithOperator : public RuleWithActions {
  public:
-    RuleWithOperator(operators::Operator *op,
+    RuleWithOperator(std::unique_ptr<operators::Operator> op,
         variables::Variables *variables,
         std::vector<actions::Action *> *actions,
         Transformations *transformations,
@@ -70,7 +67,7 @@ class RuleWithOperator : public RuleWithActions {
 
  private:
     modsecurity::variables::Variables *m_variables;
-    operators::Operator *m_operator;
+    std::unique_ptr<operators::Operator> m_operator;
 };
 
 

diff --git a/src/parser/seclang-parser.cc b/src/parser/seclang-parser.cc
@@ -2307,15 +2307,14 @@ namespace yy {
             v->push_back(i.release());
         }
 
-        Operator *op = yystack_[1].value.as < std::unique_ptr<Operator> > ().release();
-        std::unique_ptr<RuleWithOperator> rule(new RuleWithOperator(
-            /* op */ op,
+        auto rule = std::make_unique<RuleWithOperator>(
+            /* op */ std::move(yystack_[1].value.as < std::unique_ptr<Operator> > ()),
             /* variables */ v,
             /* actions */ a,
             /* transformations */ t,
             /* file name */ std::string(*yystack_[3].location.end.filename),
             /* line number */ yystack_[3].location.end.line
-            ));
+            );
 
         if (driver.addSecRule(std::move(rule)) == false) {
             YYERROR;
@@ -2332,14 +2331,14 @@ namespace yy {
             v->push_back(i.release());
         }
 
-        std::unique_ptr<RuleWithOperator> rule(new RuleWithOperator(
-            /* op */ yystack_[0].value.as < std::unique_ptr<Operator> > ().release(),
+        auto rule = std::make_unique<RuleWithOperator>(
+            /* op */ std::move(yystack_[0].value.as < std::unique_ptr<Operator> > ()),
             /* variables */ v,
-            /* actions */ NULL,
-            /* transformations */ NULL,
+            /* actions */ nullptr,
+            /* transformations */ nullptr,
             /* file name */ std::string(*yystack_[2].location.end.filename),
             /* line number */ yystack_[2].location.end.line
-            ));
+            );
         if (driver.addSecRule(std::move(rule)) == false) {
             YYERROR;
         }

diff --git a/src/parser/seclang-parser.yy b/src/parser/seclang-parser.yy
@@ -1098,15 +1098,14 @@ expression:
             v->push_back(i.release());
         }
 
-        Operator *op = $3.release();
-        std::unique_ptr<RuleWithOperator> rule(new RuleWithOperator(
-            /* op */ op,
+        auto rule = std::make_unique<RuleWithOperator>(
+            /* op */ std::move($3),
             /* variables */ v,
             /* actions */ a,
             /* transformations */ t,
             /* file name */ std::string(*@1.end.filename),
             /* line number */ @1.end.line
-            ));
+            );
 
         if (driver.addSecRule(std::move(rule)) == false) {
             YYERROR;
@@ -1119,14 +1118,14 @@ expression:
             v->push_back(i.release());
         }
 
-        std::unique_ptr<RuleWithOperator> rule(new RuleWithOperator(
-            /* op */ $3.release(),
+        auto rule = std::make_unique<RuleWithOperator>(
+            /* op */ std::move($3),
             /* variables */ v,
-            /* actions */ NULL,
-            /* transformations */ NULL,
+            /* actions */ nullptr,
+            /* transformations */ nullptr,
             /* file name */ std::string(*@1.end.filename),
             /* line number */ @1.end.line
-            ));
+            );
         if (driver.addSecRule(std::move(rule)) == false) {
             YYERROR;
         }

diff --git a/src/rule_with_operator.cc b/src/rule_with_operator.cc
@@ -23,7 +23,7 @@
 #include <cstring>
 #include <list>
 #include <utility>
-#include <memory>
+#include <cassert>
 #include <fmt/format.h>
 
 #include "modsecurity/rules_set.h"
@@ -52,22 +52,20 @@ using variables::Variable;
 using actions::transformations::None;
 
 
-RuleWithOperator::RuleWithOperator(Operator *op,
+RuleWithOperator::RuleWithOperator(std::unique_ptr<Operator> op,
     variables::Variables *_variables,
     std::vector<Action *> *actions,
     Transformations *transformations,
     const std::string &fileName,
     int lineNumber)
     : RuleWithActions(actions, transformations, std::move(fileName), lineNumber),
     m_variables(_variables),
-    m_operator(op) { /* */ }
+    m_operator(std::move(op)) {
+    assert(m_operator != nullptr);
+}
 
 
 RuleWithOperator::~RuleWithOperator() {
-    if (m_operator != NULL) {
-        delete m_operator;
-    }
-
     while (m_variables != NULL && m_variables->empty() == false) {
         auto *a = m_variables->back();
         m_variables->pop_back();