MXS-2900 MXS-2900 Clean up TestConnection string handling

Now uses std::string for several fields.

maxscale-system-test/maxtest/include/maxtest/envv.h

@@ -1,17 +1,19 @@
 #pragma once
 
-#include <stdio.h>
+#include <string>
-#include <stdarg.h>
-#include <stdlib.h>
 
 /**
- * @brief readenv Read enviromental variable, if emtpy - set dafault
+ * Read enviroment variable value. If variable is not set, set it to the given default value and return the
+ * written value.
+ *
  * @param name Name of the variable
- * @param format Default value
+ * @param format Default value format string
  * @return Enviromental variable value
  */
 char * readenv(const char * name, const char *format, ...);
 
+std::string envvar_read_write_def_str(const char* name, const char* format, ...);
+
 /**
  * @brief readenv_int Read integer value of enviromental variable, if empty - set dafault
  * @param name Name of the variable

maxscale-system-test/maxtest/include/maxtest/mariadb_nodes.h

@@ -142,12 +142,12 @@ public:
     /**
      * @brief make_snapshot_command Command line to create a snapshot of all VMs
      */
-    char* take_snapshot_command;
+    const char* take_snapshot_command;
 
     /**
      * @brief revert_snapshot_command Command line to revert a snapshot of all VMs
      */
-    char* revert_snapshot_command;
+    const char* revert_snapshot_command;
 
     int connect(int i, const std::string& db = "test");
     int connect(const std::string& db = "test");

maxscale-system-test/maxtest/include/maxtest/testconnections.h

@@ -610,18 +610,18 @@ private:
     std::string m_labels;           /**< Test labels */
     std::string m_mdbci_labels;     /**< Labels for MDBCI */
 
-    char* m_mdbci_config_name;  /**< Name of MDBCI VMs set */
+    std::string m_mdbci_config_name;  /**< Name of MDBCI VMs set */
-    char* m_mdbci_vm_path;      /**< Path to directory with MDBCI VMs descriptions */
+    std::string m_mdbci_vm_path;      /**< Path to directory with MDBCI VMs descriptions */
-    char* m_mdbci_template;     /**< Name of mdbci VMs tempate file */
+    std::string m_mdbci_template;     /**< Name of mdbci VMs tempate file */
-    char* m_target;             /**< Name of Maxscale repository in the CI */
+    std::string m_target;             /**< Name of Maxscale repository in the CI */
 
     /**
      * Command to copy log files from node virtual machines (should handle one parameter: IP address of
      * virtual machine to kill) */
-    char* m_get_logs_command;
+    std::string m_get_logs_command;
 
-    char* m_take_snapshot_command;      /**< Command line to create a snapshot of all VMs */
+    std::string m_take_snapshot_command;      /**< Command line to create a snapshot of all VMs */
-    char* m_revert_snapshot_command;    /**< Command line to revert a snapshot of all VMs */
+    std::string m_revert_snapshot_command;    /**< Command line to revert a snapshot of all VMs */
 
     bool m_enable_timeouts {true};      /**< Whether timeouts are enabled or not */
     bool m_local_maxscale {false};      /**< MaxScale runs locally, specified using -l. */

maxscale-system-test/maxtest/src/envv.cpp

@@ -1,7 +1,10 @@
-#include <string.h>
-#include <string>
 #include "envv.h"
 
+#include <cstring>
+#include <cstdarg>
+
+using std::string;
+
 char * readenv(const char * name, const char *format, ...)
 {
     char * env = getenv(name);
@@ -28,6 +31,35 @@ char * readenv(const char * name, const char *format, ...)
     return env;
 }
 
+string envvar_read_write_def_str(const char* name, const char* format, ...)
+{
+    string rval;
+    const char* old_value = getenv(name);
+    if (old_value)
+    {
+        rval = old_value;
+    }
+    else if (format)
+    {
+        va_list valist;
+        va_start(valist, format);
+        int bytes_required = vsnprintf(nullptr, 0, format, valist);
+        va_end(valist);
+
+        if (bytes_required >= 0)
+        {
+            int buflen = bytes_required + 1;
+            char buf[buflen];
+            va_start(valist, format);
+            vsnprintf(buf, buflen, format, valist);
+            va_end(valist);
+            setenv(name, buf, 1);
+            rval = buf;
+        }
+    }
+    return rval;
+}
+
 int readenv_int(const char * name, int def)
 {
     int x;

maxscale-system-test/maxtest/src/testconnections.cpp

@@ -305,8 +305,7 @@ TestConnections::TestConnections(int argc, char* argv[])
         }
     }
 
-    m_get_logs_command = (char *) malloc(strlen(test_dir) + 14);
+    m_get_logs_command = (string)test_dir + "/get_logs.sh";
-    sprintf(m_get_logs_command, "%s/get_logs.sh", test_dir);
 
     sprintf(ssl_options,
             "--ssl-cert=%s/ssl-cert/client-cert.pem --ssl-key=%s/ssl-cert/client-key.pem",
@@ -327,8 +326,8 @@ TestConnections::TestConnections(int argc, char* argv[])
     {
         repl = new Mariadb_nodes("node", test_dir, verbose, network_config);
         repl->use_ipv6 = use_ipv6;
-        repl->take_snapshot_command = m_take_snapshot_command;
+        repl->take_snapshot_command = m_take_snapshot_command.c_str();
-        repl->revert_snapshot_command = m_revert_snapshot_command;
+        repl->revert_snapshot_command = m_revert_snapshot_command.c_str();
         repl_future = std::async(std::launch::async, &Mariadb_nodes::check_nodes, repl);
     }
     else
@@ -339,10 +338,10 @@ TestConnections::TestConnections(int argc, char* argv[])
     if (!no_galera)
     {
         galera = new Galera_nodes("galera", test_dir, verbose, network_config);
-        //galera->use_ipv6 = use_ipv6;
+        // galera->use_ipv6 = use_ipv6;
         galera->use_ipv6 = false;
-        galera->take_snapshot_command = m_take_snapshot_command;
+        galera->take_snapshot_command = m_take_snapshot_command.c_str();
-        galera->revert_snapshot_command = m_revert_snapshot_command;
+        galera->revert_snapshot_command = m_revert_snapshot_command.c_str();
         galera_future = std::async(std::launch::async, &Galera_nodes::check_nodes, galera);
     }
     else
@@ -369,7 +368,7 @@ TestConnections::TestConnections(int argc, char* argv[])
 
     if (reinstall_maxscale && reinstall_maxscales())
     {
-        tprintf("Failed to install Maxscale: target is %s", m_target);
+        tprintf("Failed to install Maxscale: target is %s", m_target.c_str());
         exit(MDBCI_FAUILT);
     }
 
@@ -586,21 +585,21 @@ void TestConnections::expect(bool result, const char* format, ...)
 
 void TestConnections::read_mdbci_info()
 {
-    m_mdbci_vm_path = readenv("MDBCI_VM_PATH", "%s/vms/", getenv("HOME"));
+    m_mdbci_vm_path = envvar_read_write_def_str("MDBCI_VM_PATH", "%s/vms/", getenv("HOME"));
 
-    if (system((std::string("mkdir -p ") +
+    string cmd = "mkdir -p " + m_mdbci_vm_path;
-                std::string(m_mdbci_vm_path)).c_str()))
+    if (system(cmd.c_str()))
     {
-        tprintf("Unable to create MDBCI VMs direcory '%s', exiting", m_mdbci_vm_path);
+        tprintf("Unable to create MDBCI VMs direcory '%s', exiting", m_mdbci_vm_path.c_str());
         exit(MDBCI_FAUILT);
     }
-    m_mdbci_template = readenv("template", "default");
+    m_mdbci_template = envvar_read_write_def_str("template", "default");
-    m_target = readenv("target", "develop");
+    m_target = envvar_read_write_def_str("target", "develop");
 
-    m_mdbci_config_name = readenv("mdbci_config_name", "local");
+    m_mdbci_config_name = envvar_read_write_def_str("mdbci_config_name", "local");
-    vm_path = std::string(m_mdbci_vm_path) + "/" + std::string(m_mdbci_config_name);
+    vm_path = m_mdbci_vm_path + "/" + m_mdbci_config_name;
 
-    if (m_mdbci_config_name != NULL)
+    if (!m_mdbci_config_name.empty())
     {
         std::ifstream nc_file;
         nc_file.open(vm_path + "_network_config");
@@ -653,10 +652,12 @@ void TestConnections::read_env()
     smoke = readenv_bool("smoke", false);
     threads = readenv_int("threads", 4);
     m_use_snapshots = readenv_bool("use_snapshots", false);
-    m_take_snapshot_command = readenv("take_snapshot_command",
+    m_take_snapshot_command = envvar_read_write_def_str(
-                                      "mdbci snapshot take --path-to-nodes %s --snapshot-name ", m_mdbci_config_name);
+        "take_snapshot_command", "mdbci snapshot take --path-to-nodes %s --snapshot-name ",
-    m_revert_snapshot_command = readenv("revert_snapshot_command",
+        m_mdbci_config_name.c_str());
-                                        "mdbci snapshot revert --path-to-nodes %s --snapshot-name ", m_mdbci_config_name);
+    m_revert_snapshot_command = envvar_read_write_def_str(
+        "revert_snapshot_command", "mdbci snapshot revert --path-to-nodes %s --snapshot-name ",
+        m_mdbci_config_name.c_str());
     no_vm_revert = readenv_bool("no_vm_revert", true);
 }
 
@@ -2141,15 +2142,15 @@ void TestConnections::check_current_connections(int m, int value)
 
 int TestConnections::take_snapshot(char* snapshot_name)
 {
-    char str[strlen(m_take_snapshot_command) + strlen(snapshot_name) + 2];
+    char str[m_take_snapshot_command.length() + strlen(snapshot_name) + 2];
-    sprintf(str, "%s %s", m_take_snapshot_command, snapshot_name);
+    sprintf(str, "%s %s", m_take_snapshot_command.c_str(), snapshot_name);
     return system(str);
 }
 
 int TestConnections::revert_snapshot(char* snapshot_name)
 {
-    char str[strlen(m_revert_snapshot_command) + strlen(snapshot_name) + 2];
+    char str[m_revert_snapshot_command.length() + strlen(snapshot_name) + 2];
-    sprintf(str, "%s %s", m_revert_snapshot_command, snapshot_name);
+    sprintf(str, "%s %s", m_revert_snapshot_command.c_str(), snapshot_name);
     return system(str);
 }
 
@@ -2169,10 +2170,8 @@ bool TestConnections::test_bad_config(int m, const char* config)
 int TestConnections::call_mdbci(const char * options)
 {
     struct stat buf;
-    if (stat(
+    string filepath = m_mdbci_vm_path + "/" + m_mdbci_config_name;
-                (m_mdbci_vm_path + std::string("/") + m_mdbci_config_name).c_str(),
+    if (stat(filepath.c_str(), &buf))
-                &buf)
-       )
     {
         if (process_mdbci_template())
         {
@@ -2182,7 +2181,7 @@ int TestConnections::call_mdbci(const char * options)
         if (system((std::string("mdbci --override --template ") +
                     vm_path +
                     std::string(".json generate ") +
-                    std::string(m_mdbci_config_name)).c_str() ))
+                    m_mdbci_config_name).c_str() ))
         {
             tprintf("MDBCI failed to generate virtual machines description");
             return 1;
@@ -2199,21 +2198,19 @@ int TestConnections::call_mdbci(const char * options)
     }
 
     if (system((std::string("mdbci up ") +
-                std::string(m_mdbci_config_name) +
+               m_mdbci_config_name +
-                std::string(" --labels ") +
+               std::string(" --labels ") +
-                m_mdbci_labels +
+               m_mdbci_labels +
-                std::string(" ") +
+               std::string(" ") +
-                std::string(options)).c_str() ))
+               std::string(options)).c_str() ))
     {
         tprintf("MDBCI failed to bring up virtual machines");
         return 1;
     }
 
     std::string team_keys = readenv("team_keys", "~/.ssh/id_rsa.pub");
-    system((std::string("mdbci public_keys --key ") +
+    string cmd = "mdbci public_keys --key " + team_keys + " " + m_mdbci_config_name;
-            team_keys +
+    system(cmd.c_str());
-            std::string(" ") +
-            std::string(m_mdbci_config_name)).c_str() );
 
     read_env();
     if (repl)
@@ -2255,9 +2252,9 @@ int TestConnections::process_mdbci_template()
     }
 
     std::string name = std::string(test_dir) +
-                       std::string("/mdbci/templates/") +
+        std::string("/mdbci/templates/") +
-                       std::string(m_mdbci_template) +
+        m_mdbci_template +
-                       std::string(".json.template");
+        std::string(".json.template");
 
     std::string sys = std::string("envsubst < ") +
                       name +
@@ -2294,10 +2291,7 @@ std::string dump_status(const StringSet& current, const StringSet& expected)
 }
 int TestConnections::reinstall_maxscales()
 {
-    char sys[strlen(m_target) +
+    char sys[m_target.length() + m_mdbci_config_name.length() + strlen(maxscales->prefix) + 70];
-             strlen(m_mdbci_config_name) +
-             strlen(maxscales->prefix) +
-             70];
     for (int i = 0; i < maxscales->N; i++)
     {
         printf("Installing Maxscale on node %d\n", i);
@@ -2306,7 +2300,7 @@ int TestConnections::reinstall_maxscales()
         maxscales->ssh_node(i, "yum clean all", true);
 
         sprintf(sys, "mdbci install_product --product maxscale_ci --product-version %s %s/%s_%03d",
-                m_target, m_mdbci_config_name, maxscales->prefix, i);
+                m_target.c_str(), m_mdbci_config_name.c_str(), maxscales->prefix, i);
         if (system(sys))
         {
             return 1;