Remove unused fields from ExternalCmd

Also other cleanup.
This commit is contained in:
Esa Korhonen
2019-05-28 15:21:55 +03:00
parent e849bf261b
commit 625741d8ba
2 changed files with 30 additions and 70 deletions

View File

@ -25,10 +25,10 @@
#include <maxscale/pcre2.h> #include <maxscale/pcre2.h>
/** /**
* Tokenize a string into arguments suitable for a `execvp` call * Tokenize a string into arguments suitable for a `execvp` call.
* *
* @param args Argument string * @param argstr Argument string
* @return 0 on success, -1 on error * @return Number of arguments parsed from the string
*/ */
int ExternalCmd::tokenize_arguments(const char* argstr) int ExternalCmd::tokenize_arguments(const char* argstr)
{ {
@ -59,7 +59,7 @@ int ExternalCmd::tokenize_arguments(const char* argstr)
else if (quoted && !escaped && *ptr == qc) /** End of quoted string */ else if (quoted && !escaped && *ptr == qc) /** End of quoted string */
{ {
*ptr = '\0'; *ptr = '\0';
argv[i++] = MXS_STRDUP_A(start); argv[i++] = MXS_STRDUP(start);
read = false; read = false;
quoted = false; quoted = false;
} }
@ -70,7 +70,7 @@ int ExternalCmd::tokenize_arguments(const char* argstr)
*ptr = '\0'; *ptr = '\0';
if (read) /** New token */ if (read) /** New token */
{ {
argv[i++] = MXS_STRDUP_A(start); argv[i++] = MXS_STRDUP(start);
read = false; read = false;
} }
} }
@ -92,22 +92,20 @@ int ExternalCmd::tokenize_arguments(const char* argstr)
} }
if (read) if (read)
{ {
argv[i++] = MXS_STRDUP_A(start); argv[i++] = MXS_STRDUP(start);
} }
argv[i] = NULL; argv[i] = nullptr;
return i;
return 0;
} }
ExternalCmd* ExternalCmd::create(const char* argstr, uint32_t timeout) ExternalCmd* ExternalCmd::create(const char* argstr, int timeout)
{ {
auto cmd = new ExternalCmd; auto cmd = new ExternalCmd(timeout);
bool success = false; bool success = false;
if (argstr && cmd) if (argstr && cmd)
{ {
cmd->timeout = timeout; if (cmd->tokenize_arguments(argstr) > 0)
if (cmd->tokenize_arguments(argstr) == 0)
{ {
auto cmdname = cmd->argv[0]; auto cmdname = cmd->argv[0];
if (access(cmdname, X_OK) != 0) if (access(cmdname, X_OK) != 0)
@ -140,6 +138,11 @@ ExternalCmd* ExternalCmd::create(const char* argstr, uint32_t timeout)
return cmd; return cmd;
} }
ExternalCmd::ExternalCmd(int timeout)
: m_timeout(timeout)
{
}
ExternalCmd::~ExternalCmd() ExternalCmd::~ExternalCmd()
{ {
for (int i = 0; argv[i]; i++) for (int i = 0; argv[i]; i++)
@ -215,7 +218,6 @@ static void log_output(const char* cmd, const std::string& str)
int ExternalCmd::externcmd_execute() int ExternalCmd::externcmd_execute()
{ {
auto cmd = this;
// Create a pipe where the command can print output // Create a pipe where the command can print output
int fd[2]; int fd[2];
@ -227,7 +229,7 @@ int ExternalCmd::externcmd_execute()
int rval = 0; int rval = 0;
pid_t pid; pid_t pid;
const char* cmdname = argv[0];
// The SIGCHLD handler must be disabled before child process is forked, // The SIGCHLD handler must be disabled before child process is forked,
// otherwise we'll get an error // otherwise we'll get an error
pid = fork(); pid = fork();
@ -237,9 +239,7 @@ int ExternalCmd::externcmd_execute()
close(fd[0]); close(fd[0]);
close(fd[1]); close(fd[1]);
MXS_ERROR("Failed to execute command '%s', fork failed: [%d] %s", MXS_ERROR("Failed to execute command '%s', fork failed: [%d] %s",
cmd->argv[0], cmdname, errno, mxs_strerror(errno));
errno,
mxs_strerror(errno));
rval = -1; rval = -1;
} }
else if (pid == 0) else if (pid == 0)
@ -251,7 +251,7 @@ int ExternalCmd::externcmd_execute()
dup2(fd[1], STDERR_FILENO); dup2(fd[1], STDERR_FILENO);
// Execute the command // Execute the command
execvp(cmd->argv[0], cmd->argv); execvp(argv[0], argv);
// Close the write end of the pipe and exit // Close the write end of the pipe and exit
close(fd[1]); close(fd[1]);
@ -259,15 +259,13 @@ int ExternalCmd::externcmd_execute()
} }
else else
{ {
MXS_INFO("Executing command '%s' in process %d", cmd->argv[0], pid); MXS_INFO("Executing command '%s' in process %d", cmdname, pid);
cmd->child = pid;
cmd->n_exec++;
std::string output; std::string output;
bool first_warning = true; bool first_warning = true;
bool again = true; bool again = true;
uint64_t t = 0; uint64_t t = 0;
uint64_t t_max = cmd->timeout * 1000; uint64_t t_max = m_timeout * 1000;
// Close the write end of the pipe and make the read end non-blocking // Close the write end of the pipe and make the read end non-blocking
close(fd[1]); close(fd[1]);
@ -291,13 +289,13 @@ int ExternalCmd::externcmd_execute()
t = 0; t = 0;
if (first_warning) if (first_warning)
{ {
MXS_WARNING("Soft timeout for command '%s', sending SIGTERM", cmd->argv[0]); MXS_WARNING("Soft timeout for command '%s', sending SIGTERM", cmdname);
kill(pid, SIGTERM); kill(pid, SIGTERM);
first_warning = false; first_warning = false;
} }
else else
{ {
MXS_ERROR("Hard timeout for command '%s', sending SIGKILL", cmd->argv[0]); MXS_ERROR("Hard timeout for command '%s', sending SIGKILL", cmdname);
kill(pid, SIGKILL); kill(pid, SIGKILL);
} }
} }
@ -322,9 +320,7 @@ int ExternalCmd::externcmd_execute()
else else
{ {
rval = exit_status; rval = exit_status;
MXS_ERROR("Command '%s' did not exit normally. Exit status: %d", MXS_ERROR("Command '%s' did not exit normally. Exit status: %d", cmdname, exit_status);
cmd->argv[0],
exit_status);
} }
break; break;
} }
@ -348,7 +344,7 @@ int ExternalCmd::externcmd_execute()
{ {
std::string line = output.substr(0, pos); std::string line = output.substr(0, pos);
output.erase(0, pos + 1); output.erase(0, pos + 1);
log_output(cmd->argv[0], line); log_output(cmdname, line);
} }
} }
} }
@ -356,7 +352,7 @@ int ExternalCmd::externcmd_execute()
if (!output.empty()) if (!output.empty())
{ {
log_output(cmd->argv[0], output); log_output(cmdname, output);
} }
// Close the read end of the pipe and copy the data to the output parameter // Close the read end of the pipe and copy the data to the output parameter
@ -451,30 +447,6 @@ static char* get_command(const char* str)
return rval; return rval;
} }
bool externcmd_can_execute(const char* argstr)
{
bool rval = false;
char* command = get_command(argstr);
if (command)
{
if (access(command, X_OK) == 0)
{
rval = true;
}
else if (access(command, F_OK) == 0)
{
MXS_ERROR("The executable cannot be executed: %s", command);
}
else
{
MXS_ERROR("The executable cannot be found: %s", command);
}
MXS_FREE(command);
}
return rval;
}
bool ExternalCmd::externcmd_matches(const char* match) bool ExternalCmd::externcmd_matches(const char* match)
{ {
for (int i = 0; argv[i]; i++) for (int i = 0; argv[i]; i++)

View File

@ -22,11 +22,11 @@ public:
* Create a new external command. The name and parameters are copied so * Create a new external command. The name and parameters are copied so
* the original memory can be freed. * the original memory can be freed.
* *
* @param command Command to execute with the parameters * @param argstr Command to execute with the parameters
* @param timeout Command timeout in seconds * @param timeout Command timeout in seconds
* @return Pointer to new external command struct or NULL if an error occurred * @return Pointer to new external command struct or NULL if an error occurred
*/ */
static ExternalCmd* create(const char* argstr, uint32_t timeout); static ExternalCmd* create(const char* argstr, int timeout);
~ExternalCmd(); ~ExternalCmd();
@ -63,21 +63,9 @@ public:
char* argv[MAX_ARGS + 1] {}; /**< Arguments. First element is the command. */ char* argv[MAX_ARGS + 1] {}; /**< Arguments. First element is the command. */
private: private:
int n_exec; /**< Number of times executed */ int m_timeout; /**< Command timeout in seconds */
pid_t child; /**< PID of the child process */
uint32_t timeout; /**< Command timeout in seconds */
ExternalCmd(int timeout);
int tokenize_arguments(const char* argstr); int tokenize_arguments(const char* argstr);
}; };
/**
* Check if a command can be executed
*
* Checks if the file being executed exists and if the current user has execution
* permissions on the file.
*
* @param argstr Command to check, can contain arguments for the command
*
* @return True if the file was found and the use has execution permissions to it
*/
bool externcmd_can_execute(const char* argstr);