Clean up external command argument substitution

Use a single function for checking match and for replacing.

server/core/externcmd.cc

@@ -33,8 +33,8 @@ int ExternalCmd::tokenize_args(char* dest[], int dest_size)
     bool escaped = false;
     char qc = 0;
 
-    char args[m_command_substituted.length() + 1];
+    char args[m_subst_command.length() + 1];
-    strcpy(args, m_command_substituted.c_str());
+    strcpy(args, m_subst_command.c_str());
     char* start = args;
     char* ptr = start;
     int i = 0;
@@ -130,8 +130,8 @@ std::unique_ptr<ExternalCmd> ExternalCmd::create(const string& argstr, int timeo
 }
 
 ExternalCmd::ExternalCmd(const std::string& script, int timeout)
-    : m_command(script)
+    : m_orig_command(script)
-    , m_command_substituted(script)
+    , m_subst_command(script)
     , m_timeout(timeout)
 {
 }
@@ -357,16 +357,16 @@ void ExternalCmd::substitute_arg(const std::string& match, const std::string& re
 {
     // The match may be in the subject multiple times. Find all locations.
     string::size_type next_search_begin = 0;
-    while (next_search_begin < m_command_substituted.length())
+    while (next_search_begin < m_subst_command.length())
     {
-        auto position = m_command_substituted.find(match, next_search_begin);
+        auto position = m_subst_command.find(match, next_search_begin);
         if (position == string::npos)
         {
-            next_search_begin = m_command_substituted.length();
+            next_search_begin = m_subst_command.length();
         }
         else
         {
-            m_command_substituted.replace(position, match.length(), replace);
+            m_subst_command.replace(position, match.length(), replace);
             next_search_begin = position + replace.length();
         }
     }
@@ -412,17 +412,25 @@ static char* get_command(const char* str)
     return rval;
 }
 
-bool ExternalCmd::externcmd_matches(const char* match)
+bool ExternalCmd::externcmd_matches(const string& match)
 {
-    return m_command.find(match) != string::npos;
+    return m_orig_command.find(match) != string::npos;
+}
+
+void ExternalCmd::match_substitute(const std::string& keyword, std::function<std::string(void)> generator)
+{
+    if (externcmd_matches(keyword))
+    {
+        substitute_arg(keyword, generator());
+    }
 }
 
 void ExternalCmd::reset_substituted()
 {
-    m_command_substituted = m_command;
+    m_subst_command = m_orig_command;
 }
 
 const char* ExternalCmd::substituted() const
 {
-    return m_command_substituted.c_str();
+    return m_subst_command.c_str();
 }

server/core/internal/externcmd.hh

@@ -13,8 +13,9 @@
 #pragma once
 
 #include <maxscale/ccdefs.hh>
-#include <unistd.h>
+#include <functional>
 #include <memory>
+#include <unistd.h>
 
 class ExternalCmd
 {
@@ -50,10 +51,17 @@ public:
      * Simple matching of string and command
      *
      * @param match String to search for
-     *
      * @return True if the string matched
      */
-    bool externcmd_matches(const char* match);
+    bool externcmd_matches(const std::string& match);
+
+    /**
+     * If keyword is found in command script, replace keyword with output of generator function.
+     *
+     * @param keyword Keyword to replace
+     * @param generator Function which generates the replacement string. Only ran if keyword was found.
+     */
+    void match_substitute(const std::string& keyword, std::function<std::string(void)> generator);
 
     void reset_substituted();
 
@@ -62,9 +70,9 @@ public:
 private:
     static const int MAX_ARGS {256};
 
-    std::string m_command;              /**< Original command */
+    std::string m_orig_command;        /**< Original command */
-    std::string m_command_substituted;  /**< Command with substitutions */
+    std::string m_subst_command;       /**< Command with substitutions */
-    int         m_timeout;              /**< Command timeout in seconds */
+    int         m_timeout;             /**< Command timeout in seconds */
 
     ExternalCmd(const std::string& script, int timeout);
 

server/core/monitor.cc

@@ -42,6 +42,7 @@
 #include <maxscale/utils.hh>
 #include <maxscale/json_api.hh>
 #include <mysqld_error.h>
+#include <maxbase/format.hh>
 
 #include "internal/config.hh"
 #include "internal/externcmd.hh"
@@ -1071,37 +1072,34 @@ std::string Monitor::child_nodes(MonitorServer* parent)
 
 int Monitor::launch_command(MonitorServer* ptr)
 {
+    // A generator function is ran only if the matching substitution keyword is found.
+
+    auto gen_initiator = [ptr] {
+            return mxb::string_printf("[%s]:%d", ptr->server->address, ptr->server->port);
+        };
+
+    auto gen_parent = [this, ptr] {
+            string ss;
+            MonitorServer* parent = find_parent_node(ptr);
+            if (parent)
+            {
+                ss = mxb::string_printf("[%s]:%d", parent->server->address, parent->server->port);
+            }
+            return ss;
+        };
+
     auto cmd = m_scriptcmd.get();
     cmd->reset_substituted();
+    cmd->match_substitute("$INITIATOR", gen_initiator);
+    cmd->match_substitute("$PARENT", gen_parent);
 
-    if (cmd->externcmd_matches("$INITIATOR"))
+    cmd->match_substitute("$CHILDREN", [this, ptr] {
-    {
+                              return child_nodes(ptr);
-        char initiator[strlen(ptr->server->address) + 24];      // Extra space for port
+                          });
-        snprintf(initiator, sizeof(initiator), "[%s]:%d", ptr->server->address, ptr->server->port);
-        cmd->substitute_arg("$INITIATOR", initiator);
-    }
 
-    if (cmd->externcmd_matches("$PARENT"))
+    cmd->match_substitute("$EVENT", [ptr] {
-    {
+                              return ptr->get_event_name();
-        std::stringstream ss;
+                          });
-        MonitorServer* parent = find_parent_node(ptr);
-
-        if (parent)
-        {
-            ss << "[" << parent->server->address << "]:" << parent->server->port;
-        }
-        cmd->substitute_arg("$PARENT", ss.str().c_str());
-    }
-
-    if (cmd->externcmd_matches("$CHILDREN"))
-    {
-        cmd->substitute_arg("$CHILDREN", child_nodes(ptr).c_str());
-    }
-
-    if (cmd->externcmd_matches("$EVENT"))
-    {
-        cmd->substitute_arg("$EVENT", ptr->get_event_name());
-    }
 
     char nodelist[PATH_MAX + MON_ARG_MAX + 1] = {'\0'};