Use shared_ptr instead of new/delete to prevent memory leak

sonic-net · Aug 2, 2023 · 7c161e2 · 7c161e2
1 parent b0b587a
commit 7c161e2
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 16 deletions.
diff --git a/dash-pipeline/SAI/templates/saiapi.cpp.j2 b/dash-pipeline/SAI/templates/saiapi.cpp.j2
@@ -14,7 +14,7 @@ static sai_status_t dash_sai_create_{{ table.name }}(
 {
     DASH_LOG_ENTER();
 
-    p4::v1::TableEntry * matchActionEntry = nullptr;
+    std::shared_ptr<p4::v1::TableEntry> matchActionEntry;
     pi_p4_id_t tableId = 0;
     // There shall be one and only one action_type
     p4::v1::TableAction* entry = nullptr;
@@ -31,7 +31,7 @@ static sai_status_t dash_sai_create_{{ table.name }}(
     {% if 'stage' in table %}
     // For stage {{ table.stage }}
     {% endif %}
-    matchActionEntry = new p4::v1::TableEntry();
+    matchActionEntry = std::make_shared<p4::v1::TableEntry>();
     tableId = {{table.id}};
     entry = matchActionEntry->mutable_action();
     action = entry->mutable_action();
@@ -179,7 +179,6 @@ static sai_status_t dash_sai_create_{{ table.name }}(
     *{{ table.name }}_id = objId;
     return SAI_STATUS_SUCCESS;
 ErrRet:
-    delete matchActionEntry;
     RemoveFromTable(*{{ table.name }}_id);
     return SAI_STATUS_FAILURE;
 }
@@ -275,7 +274,7 @@ static sai_status_t dash_sai_create_{{ table.name }}(
 {
     DASH_LOG_ENTER();
 
-    p4::v1::TableEntry * matchActionEntry = new p4::v1::TableEntry();
+    std::shared_ptr<p4::v1::TableEntry> matchActionEntry = std::make_shared<p4::v1::TableEntry>();
     pi_p4_id_t tableId = {{table.id}};
     matchActionEntry->set_table_id(tableId);
     auto tableEntry = {{ table.name }};
@@ -379,11 +378,9 @@ static sai_status_t dash_sai_create_{{ table.name }}(
     // TODO: ternaly needs to set priority
     retCode = MutateTableEntry(matchActionEntry, p4::v1::Update_Type_INSERT);
     if (grpc::StatusCode::OK == retCode) {
-        delete matchActionEntry;
         return SAI_STATUS_SUCCESS;
     }
 ErrRet:
-    delete matchActionEntry;
     return SAI_STATUS_FAILURE;
 }
 
@@ -420,7 +417,7 @@ static sai_status_t dash_sai_remove_{{ table.name }}(
 {
     DASH_LOG_ENTER();
 
-    p4::v1::TableEntry * matchActionEntry = new p4::v1::TableEntry();
+    std::shared_ptr<p4::v1::TableEntry> matchActionEntry = std::make_shared<p4::v1::TableEntry>();
     pi_p4_id_t tableId = {{table.id}};
     matchActionEntry->set_table_id(tableId);
     auto tableEntry =  {{ table.name }};
@@ -461,13 +458,11 @@ static sai_status_t dash_sai_remove_{{ table.name }}(
 
     retCode = MutateTableEntry(matchActionEntry, p4::v1::Update_Type_DELETE);
     if (grpc::StatusCode::OK == retCode) {
-        delete matchActionEntry;
         return SAI_STATUS_SUCCESS;
     }
 
 ErrRet:
 
-    delete matchActionEntry;
     return SAI_STATUS_FAILURE;
 }
 

diff --git a/dash-pipeline/SAI/templates/utils.cpp.j2 b/dash-pipeline/SAI/templates/utils.cpp.j2
@@ -2,7 +2,7 @@
 
 static int deviceId = 0;
 
-static std::unordered_multimap<sai_object_id_t, p4::v1::TableEntry *> tableEntryMap;
+static std::unordered_multimap<sai_object_id_t, std::shared_ptr<p4::v1::TableEntry> > tableEntryMap;
 static std::mutex tableLock;
 static std::atomic<sai_object_id_t> nextId;
 
@@ -64,7 +64,7 @@ const sai_attribute_t *getMaskAttr(sai_attr_id_t id, uint32_t attr_count, const
     return nullptr;
 }
 
-grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type updateType)
+grpc::StatusCode MutateTableEntry(std::shared_ptr<p4::v1::TableEntry> entry, p4::v1::Update_Type updateType)
 {
     DASH_LOG_ENTER();
 
@@ -73,7 +73,7 @@ grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type
     auto update = request.add_updates();
     update->set_type(updateType);
     auto entity = update->mutable_entity();
-    entity->set_allocated_table_entry(entry);
+    entity->set_allocated_table_entry(entry.get());
 
     p4::v1::WriteResponse rep;
     grpc::ClientContext context;
@@ -94,7 +94,7 @@ grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type
     return status.error_code();
 }
 
-bool InsertInTable(p4::v1::TableEntry *entry, sai_object_id_t *objId)
+bool InsertInTable(std::shared_ptr<p4::v1::TableEntry> entry, sai_object_id_t *objId)
 {
     DASH_LOG_ENTER();
 
@@ -141,7 +141,6 @@ bool RemoveFromTable(sai_object_id_t id)
         if (grpc::StatusCode::OK != tempRet) {
             retCode = tempRet;
         }
-        delete entry;
     }
 
     tableEntryMap.erase(id);

diff --git a/dash-pipeline/SAI/templates/utils.h.j2 b/dash-pipeline/SAI/templates/utils.h.j2
@@ -26,6 +26,7 @@ extern "C"
 #include <atomic>
 #include <limits>
 #include <fstream>
+#include <memory>
 
 template<typename T>
 void booldataSetVal(const sai_attribute_value_t &value, T &t, int bits = 8){
@@ -310,11 +311,11 @@ void u64SetMask(const sai_uint64_t &value, T &t, int bits = 64) {
     t->set_mask(&val, bytes);
 }
 
-grpc::StatusCode MutateTableEntry(p4::v1::TableEntry *entry, p4::v1::Update_Type updateType);
+grpc::StatusCode MutateTableEntry(std::shared_ptr<p4::v1::TableEntry>, p4::v1::Update_Type updateType);
 
 sai_object_id_t NextObjIndex();
 
-bool InsertInTable(p4::v1::TableEntry *entry, sai_object_id_t *objId);
+bool InsertInTable(std::shared_ptr<p4::v1::TableEntry> entry, sai_object_id_t *objId);
 
 bool RemoveFromTable(sai_object_id_t id);