From e849bf261bcff3381a3a359ad27c2f3be4e83ccc Mon Sep 17 00:00:00 2001 From: Esa Korhonen Date: Fri, 24 May 2019 11:28:52 +0300 Subject: [PATCH] Move ExternalCmd-functions to class methods --- server/core/externcmd.cc | 78 +++++++++++++------------------ server/core/internal/externcmd.hh | 67 ++++++++++++-------------- server/core/monitor.cc | 46 +++++++++--------- 3 files changed, 85 insertions(+), 106 deletions(-) diff --git a/server/core/externcmd.cc b/server/core/externcmd.cc index d1e63d9cd..6395811ae 100644 --- a/server/core/externcmd.cc +++ b/server/core/externcmd.cc @@ -18,7 +18,6 @@ #include #include #include -#include #include #include @@ -29,11 +28,9 @@ * Tokenize a string into arguments suitable for a `execvp` call * * @param args Argument string - * @param argv Array of char pointers to be filled with tokenized arguments - * * @return 0 on success, -1 on error */ -static int tokenize_arguments(const char* argstr, char** argv) +int ExternalCmd::tokenize_arguments(const char* argstr) { int i = 0; bool quoted = false; @@ -47,7 +44,7 @@ static int tokenize_arguments(const char* argstr, char** argv) start = args; ptr = start; - while (*ptr != '\0' && i < MAXSCALE_EXTCMD_ARG_MAX) + while (*ptr != '\0' && i < MAX_ARGS) { if (escaped) { @@ -103,60 +100,51 @@ static int tokenize_arguments(const char* argstr, char** argv) return 0; } -ExternalCmd* ExternalCmd::externcmd_allocate(const char* argstr, uint32_t timeout) +ExternalCmd* ExternalCmd::create(const char* argstr, uint32_t timeout) { - ExternalCmd* cmd = (ExternalCmd*) MXS_MALLOC(sizeof(ExternalCmd)); - char** argv = (char**) MXS_MALLOC(sizeof(char*) * MAXSCALE_EXTCMD_ARG_MAX); - - if (argstr && cmd && argv) + auto cmd = new ExternalCmd; + bool success = false; + if (argstr && cmd) { cmd->timeout = timeout; - cmd->argv = argv; - if (tokenize_arguments(argstr, cmd->argv) == 0) + if (cmd->tokenize_arguments(argstr) == 0) { - if (access(cmd->argv[0], X_OK) != 0) + auto cmdname = cmd->argv[0]; + if (access(cmdname, X_OK) != 0) { - if (access(cmd->argv[0], F_OK) != 0) + if (access(cmdname, F_OK) != 0) { - MXS_ERROR("Cannot find file: %s", cmd->argv[0]); + MXS_ERROR("Cannot find file: %s", cmdname); } else { - MXS_ERROR("Cannot execute file '%s'. Missing " - "execution permissions.", - cmd->argv[0]); + MXS_ERROR("Cannot execute file '%s'. Missing execution permissions.", cmdname); } - externcmd_free(cmd); - cmd = NULL; + } + else + { + success = true; } } else { - MXS_ERROR("Failed to parse argument string for external command: %s", - argstr); - externcmd_free(cmd); - cmd = NULL; + MXS_ERROR("Failed to parse argument string for external command: %s", argstr); } } - else + + if (!success) { - MXS_FREE(cmd); - MXS_FREE(argv); - cmd = NULL; + delete cmd; + cmd = nullptr; } return cmd; } -void ExternalCmd::externcmd_free(ExternalCmd* cmd) +ExternalCmd::~ExternalCmd() { - if (cmd) + for (int i = 0; argv[i]; i++) { - for (int i = 0; cmd->argv[i]; i++) - { - MXS_FREE(cmd->argv[i]); - } - MXS_FREE(cmd->argv); - MXS_FREE(cmd); + MXS_FREE(argv[i]); } } @@ -378,7 +366,7 @@ int ExternalCmd::externcmd_execute() return rval; } -bool externcmd_substitute_arg(ExternalCmd* cmd, const char* match, const char* replace) +bool ExternalCmd::substitute_arg(const char* match, const char* replace) { int err; bool rval = true; @@ -386,15 +374,15 @@ bool externcmd_substitute_arg(ExternalCmd* cmd, const char* match, const char* r pcre2_code* re = pcre2_compile((PCRE2_SPTR) match, PCRE2_ZERO_TERMINATED, 0, &err, &errpos, NULL); if (re) { - for (int i = 0; cmd->argv[i] && rval; i++) + for (int i = 0; argv[i] && rval; i++) { - size_t size_orig = strlen(cmd->argv[i]); + size_t size_orig = strlen(argv[i]); size_t size_replace = strlen(replace); size_t size = MXS_MAX(size_orig, size_replace); char* dest = (char*)MXS_MALLOC(size); if (dest) { - mxs_pcre2_result_t rc = mxs_pcre2_substitute(re, cmd->argv[i], replace, &dest, &size); + mxs_pcre2_result_t rc = mxs_pcre2_substitute(re, argv[i], replace, &dest, &size); switch (rc) { @@ -404,8 +392,8 @@ bool externcmd_substitute_arg(ExternalCmd* cmd, const char* match, const char* r break; case MXS_PCRE2_MATCH: - MXS_FREE(cmd->argv[i]); - cmd->argv[i] = dest; + MXS_FREE(argv[i]); + argv[i] = dest; break; case MXS_PCRE2_NOMATCH: @@ -487,11 +475,11 @@ bool externcmd_can_execute(const char* argstr) return rval; } -bool externcmd_matches(const ExternalCmd* cmd, const char* match) +bool ExternalCmd::externcmd_matches(const char* match) { - for (int i = 0; cmd->argv[i]; i++) + for (int i = 0; argv[i]; i++) { - if (strstr(cmd->argv[i], match)) + if (strstr(argv[i], match)) { return true; } diff --git a/server/core/internal/externcmd.hh b/server/core/internal/externcmd.hh index c3c7afef5..f9b79849e 100644 --- a/server/core/internal/externcmd.hh +++ b/server/core/internal/externcmd.hh @@ -15,30 +15,20 @@ #include #include -#define MAXSCALE_EXTCMD_ARG_MAX 256 - class ExternalCmd { public: /** - * Allocate a new external command. - * - * The name and parameters are copied into the external command structure so - * the original memory can be freed if needed. + * Create a new external command. The name and parameters are copied so + * the original memory can be freed. * * @param command Command to execute with the parameters * @param timeout Command timeout in seconds - * * @return Pointer to new external command struct or NULL if an error occurred */ - static ExternalCmd* externcmd_allocate(const char* argstr, uint32_t timeout); + static ExternalCmd* create(const char* argstr, uint32_t timeout); - /** - * Free a previously allocated external command - * - * @param cmd Command to free - */ - static void externcmd_free(ExternalCmd* cmd); + ~ExternalCmd(); /** * Execute a command @@ -49,23 +39,36 @@ public: */ int externcmd_execute(); - char** argv; /**< Argument vector for the command, first being the - * actual command being executed */ + /** + * Substitute all occurrences of @c match with @c replace in the arguments. + * + * @param match Match string + * @param replace Replacement string + * + * @return True if replacement was successful, false on error + */ + bool substitute_arg(const char* match, const char* replace); + + /** + * Simple matching of string and command + * + * @param match String to search for + * + * @return True if the string matched + */ + bool externcmd_matches(const char* match); + + static const int MAX_ARGS {256}; + + char* argv[MAX_ARGS + 1] {}; /**< Arguments. First element is the command. */ + +private: int n_exec; /**< Number of times executed */ pid_t child; /**< PID of the child process */ uint32_t timeout; /**< Command timeout in seconds */ -}; -/** - * Substitute all occurrences of @c match with @c replace in the arguments for @c cmd - * - * @param cmd External command - * @param match Match string - * @param replace Replacement string - * - * @return True if replacement was successful, false on error - */ -bool externcmd_substitute_arg(ExternalCmd* cmd, const char* re, const char* replace); + int tokenize_arguments(const char* argstr); +}; /** * Check if a command can be executed @@ -78,13 +81,3 @@ bool externcmd_substitute_arg(ExternalCmd* cmd, const char* re, const char* repl * @return True if the file was found and the use has execution permissions to it */ bool externcmd_can_execute(const char* argstr); - -/** - * Simple matching of string and command - * - * @param cmd Command where the match is searched from - * @param match String to search for - * - * @return True if the string matched - */ -bool externcmd_matches(const ExternalCmd* cmd, const char* match); diff --git a/server/core/monitor.cc b/server/core/monitor.cc index 30b44768d..96fe41982 100644 --- a/server/core/monitor.cc +++ b/server/core/monitor.cc @@ -1056,14 +1056,14 @@ std::string Monitor::child_nodes(MonitorServer* parent) int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) { - if (externcmd_matches(cmd, "$INITIATOR")) + if (cmd->externcmd_matches("$INITIATOR")) { char initiator[strlen(ptr->server->address) + 24]; // Extra space for port snprintf(initiator, sizeof(initiator), "[%s]:%d", ptr->server->address, ptr->server->port); - externcmd_substitute_arg(cmd, "[$]INITIATOR", initiator); + cmd->substitute_arg("[$]INITIATOR", initiator); } - if (externcmd_matches(cmd, "$PARENT")) + if (cmd->externcmd_matches("$PARENT")) { std::stringstream ss; MonitorServer* parent = find_parent_node(ptr); @@ -1072,56 +1072,56 @@ int Monitor::launch_command(MonitorServer* ptr, ExternalCmd* cmd) { ss << "[" << parent->server->address << "]:" << parent->server->port; } - externcmd_substitute_arg(cmd, "[$]PARENT", ss.str().c_str()); + cmd->substitute_arg("[$]PARENT", ss.str().c_str()); } - if (externcmd_matches(cmd, "$CHILDREN")) + if (cmd->externcmd_matches("$CHILDREN")) { - externcmd_substitute_arg(cmd, "[$]CHILDREN", child_nodes(ptr).c_str()); + cmd->substitute_arg("[$]CHILDREN", child_nodes(ptr).c_str()); } - if (externcmd_matches(cmd, "$EVENT")) + if (cmd->externcmd_matches("$EVENT")) { - externcmd_substitute_arg(cmd, "[$]EVENT", ptr->get_event_name()); + cmd->substitute_arg("[$]EVENT", ptr->get_event_name()); } char nodelist[PATH_MAX + MON_ARG_MAX + 1] = {'\0'}; - if (externcmd_matches(cmd, "$CREDENTIALS")) + if (cmd->externcmd_matches("$CREDENTIALS")) { // We provide the credentials for _all_ servers. append_node_names(nodelist, sizeof(nodelist), 0, CredentialsApproach::INCLUDE); - externcmd_substitute_arg(cmd, "[$]CREDENTIALS", nodelist); + cmd->substitute_arg("[$]CREDENTIALS", nodelist); } - if (externcmd_matches(cmd, "$NODELIST")) + if (cmd->externcmd_matches("$NODELIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_RUNNING); - externcmd_substitute_arg(cmd, "[$]NODELIST", nodelist); + cmd->substitute_arg("[$]NODELIST", nodelist); } - if (externcmd_matches(cmd, "$LIST")) + if (cmd->externcmd_matches("$LIST")) { append_node_names(nodelist, sizeof(nodelist), 0); - externcmd_substitute_arg(cmd, "[$]LIST", nodelist); + cmd->substitute_arg("[$]LIST", nodelist); } - if (externcmd_matches(cmd, "$MASTERLIST")) + if (cmd->externcmd_matches("$MASTERLIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_MASTER); - externcmd_substitute_arg(cmd, "[$]MASTERLIST", nodelist); + cmd->substitute_arg("[$]MASTERLIST", nodelist); } - if (externcmd_matches(cmd, "$SLAVELIST")) + if (cmd->externcmd_matches("$SLAVELIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_SLAVE); - externcmd_substitute_arg(cmd, "[$]SLAVELIST", nodelist); + cmd->substitute_arg("[$]SLAVELIST", nodelist); } - if (externcmd_matches(cmd, "$SYNCEDLIST")) + if (cmd->externcmd_matches("$SYNCEDLIST")) { append_node_names(nodelist, sizeof(nodelist), SERVER_JOINED); - externcmd_substitute_arg(cmd, "[$]SYNCEDLIST", nodelist); + cmd->substitute_arg("[$]SYNCEDLIST", nodelist); } int rv = cmd->externcmd_execute(); @@ -1202,7 +1202,7 @@ int Monitor::launch_script(MonitorServer* ptr) char arg[strlen(script) + 1]; strcpy(arg, script); - ExternalCmd* cmd = ExternalCmd::externcmd_allocate(arg, m_settings.script_timeout); + ExternalCmd* cmd = ExternalCmd::create(arg, m_settings.script_timeout); if (cmd == NULL) { @@ -1213,9 +1213,7 @@ int Monitor::launch_script(MonitorServer* ptr) } int rv = launch_command(ptr, cmd); - - ExternalCmd::externcmd_free(cmd); - + delete cmd; return rv; }