harden smem export

Harden the logic for exporting smem to prevent segfaults. Previously we had a segfault with just `smem -x file.soar` because the smem DB is not connected. Adding an LTI option, however, would connect to the DB first. Change it so that export (as with backup) requires connecting to the DB first, and gracefully returns false if not connected.
SoarGroup · Aug 23, 2023 · 343e367 · 343e367
1 parent 39d3d66
commit 343e367
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 41 deletions.
diff --git a/Core/CLI/src/cli_load_save.cpp b/Core/CLI/src/cli_load_save.cpp
@@ -212,6 +212,9 @@ bool CommandLineInterface::DoSave(std::vector<std::string>& argv, const std::str
             if (thisAgent->SMem->connected() && (thisAgent->SMem->statistics->nodes->get_value() > 0))
             {
                 result = thisAgent->SMem->export_smem(0, export_text, &(err));
+                if (!result) {
+                    SetError(*err);
+                }
                 AddSaveText("# Semantic Memory\n");
                 if (!DoCLog(LOG_ADD, 0, &export_text, true)) return false;
             } else {

diff --git a/Core/CLI/src/cli_smem.cpp b/Core/CLI/src/cli_smem.cpp
@@ -403,14 +403,13 @@ bool CommandLineInterface::DoSMem(const char pOp, const std::string* pArg1, cons
         std::string* err = new std::string("");
         uint64_t lti_id = NIL;
 
-        if (pArg2)
+        if (!thisAgent->SMem->connected())
         {
-            thisAgent->SMem->attach();
-            if (!thisAgent->SMem->connected())
-            {
-                return SetError("Semantic memory database not connected.");
-            }
+            return SetError("Cannot export smem: smem database not connected.");
+        }
 
+        if (pArg2)
+        {
             const char* pAttr_c_str = pArg2->c_str();
             soar::Lexer lexer(thisAgent, pAttr_c_str);
             if (!lexer.get_lexeme()) return SetError("LTI not found.");
@@ -456,41 +455,8 @@ bool CommandLineInterface::DoSMem(const char pOp, const std::string* pArg1, cons
                 return false;
             }
 
-            PrintCLIMessage("Exported semantic memory to file.");
-        }
-        delete err;
-        return result;
-    }
-    else if (pOp == 'x')
-    {
-        std::string* err = new std::string("smem_export.soar");
-        uint64_t lti_id = NIL;
-
-        std::string export_text;
-        bool result = thisAgent->SMem->export_smem(0, export_text, &(err));
-
-        if (!result)
-        {
-            SetError(*err);
-        }
-        else
-        {
-            if (!DoCLog(LOG_NEW, err, 0, true))
-            {
-                return false;
-            }
-
-            if (!DoCLog(LOG_ADD, 0, &export_text, true))
-            {
-                return false;
-            }
-
-            if (!DoCLog(LOG_CLOSE, 0, 0, true))
-            {
-                return false;
-            }
-
-            PrintCLIMessage("Exported semantic memory to file.");
+            tempString << "Exported semantic memory to file '" << pArg1->c_str() << "'.";
+            PrintCLIMessage(&tempString);
         }
         delete err;
         return result;

diff --git a/Core/SoarKernel/src/semantic_memory/smem_print.cpp b/Core/SoarKernel/src/semantic_memory/smem_print.cpp
@@ -16,6 +16,10 @@
 bool SMem_Manager::export_smem(uint64_t lti_id, std::string& result_text, std::string** err_msg)
 {
     ltm_set store_set;
+    if (!connected()) {
+        (*err_msg)->append("Cannot export semantic memory if it is not connected.");
+        return false;
+    }
 
     if (!lti_id)
     {