diff options
author | Costa Tsaousis <costa@netdata.cloud> | 2022-04-28 18:35:08 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-04-28 18:35:08 +0300 |
commit | ae75f49f686a73a348396261e1434488f6360bd2 (patch) | |
tree | f3bf2d84ac23861be42e863d8a194e58cfdfdc76 /libnetdata | |
parent | dd24ca29026dc568358613115bfed86fe6a75d4b (diff) |
faster execution of external programs (#12759)
* faster invocation of external plugins by eliminating the need for starting /bin/sh and then the command
* added missing parameter
* prefer the z function
* cleanup and clarity - addressed LGTM issue
* simplified the popen() interface a bit, to make it more predictable for future uses
* removed commented old code
* more comments cleanup
* mypopen_raw() added for completeness - it is not currently used
* simplified the mypopen_raw() interface even further
* Update libnetdata/popen/popen.c
Co-authored-by: Vladimir Kobal <vlad@prokk.net>
* restored 0 flags for netdata_spawn() and cosmetic changes
* added more clarity to the code and reverted old behavior of all other execution of commands
Co-authored-by: Vladimir Kobal <vlad@prokk.net>
Diffstat (limited to 'libnetdata')
-rw-r--r-- | libnetdata/popen/popen.c | 108 | ||||
-rw-r--r-- | libnetdata/popen/popen.h | 17 |
2 files changed, 101 insertions, 24 deletions
diff --git a/libnetdata/popen/popen.c b/libnetdata/popen/popen.c index 33f4bd950c..fa9dd2b6c0 100644 --- a/libnetdata/popen/popen.c +++ b/libnetdata/popen/popen.c @@ -78,28 +78,34 @@ static void myp_del(pid_t pid) { #define PIPE_READ 0 #define PIPE_WRITE 1 -/* custom_popene flag definitions */ -#define FLAG_CREATE_PIPE 1 // Create a pipe like popen() when set, otherwise set stdout to /dev/null -#define FLAG_CLOSE_FD 2 // Close all file descriptors other than STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO +static inline void convert_argv_to_string(char *dst, size_t size, const char *spawn_argv[]) { + int i; + for(i = 0; spawn_argv[i] ;i++) { + if(i == 0) snprintfz(dst, size, "%s", spawn_argv[i]); + else { + size_t len = strlen(dst); + snprintfz(&dst[len], size - len, " '%s'", spawn_argv[i]); + } + } +} /* - * Returns -1 on failure, 0 on success. When FLAG_CREATE_PIPE is set, on success set the FILE *fp pointer. + * Returns -1 on failure, 0 on success. When POPEN_FLAG_CREATE_PIPE is set, on success set the FILE *fp pointer. */ -static inline int custom_popene(const char *command, volatile pid_t *pidptr, char **env, uint8_t flags, FILE **fpp) { +static int custom_popene(volatile pid_t *pidptr, char **env, uint8_t flags, FILE **fpp, const char *command, const char *spawn_argv[]) { + // create a string to be logged about the command we are running + char command_to_be_logged[2048]; + convert_argv_to_string(command_to_be_logged, sizeof(command_to_be_logged), spawn_argv); + // info("custom_popene() running command: %s", command_to_be_logged); + FILE *fp = NULL; int ret = 0; // success by default int pipefd[2], error; pid_t pid; - char *const spawn_argv[] = { - "sh", - "-c", - (char *)command, - NULL - }; posix_spawnattr_t attr; posix_spawn_file_actions_t fa; - if (flags & FLAG_CREATE_PIPE) { + if (flags & POPEN_FLAG_CREATE_PIPE) { if (pipe(pipefd) == -1) return -1; if ((fp = fdopen(pipefd[PIPE_READ], "r")) == NULL) { @@ -107,7 +113,7 @@ static inline int custom_popene(const char *command, volatile pid_t *pidptr, cha } } - if (flags & FLAG_CLOSE_FD) { + if (flags & POPEN_FLAG_CLOSE_FD) { // Mark all files to be closed by the exec() stage of posix_spawn() int i; for (i = (int) (sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--) { @@ -117,7 +123,7 @@ static inline int custom_popene(const char *command, volatile pid_t *pidptr, cha } if (!posix_spawn_file_actions_init(&fa)) { - if (flags & FLAG_CREATE_PIPE) { + if (flags & POPEN_FLAG_CREATE_PIPE) { // move the pipe to stdout in the child if (posix_spawn_file_actions_adddup2(&fa, pipefd[PIPE_WRITE], STDOUT_FILENO)) { error("posix_spawn_file_actions_adddup2() failed"); @@ -150,22 +156,22 @@ static inline int custom_popene(const char *command, volatile pid_t *pidptr, cha // Take the lock while we fork to ensure we don't race with SIGCHLD // delivery on a process which exits quickly. myp_add_lock(); - if (!posix_spawn(&pid, "/bin/sh", &fa, &attr, spawn_argv, env)) { + if (!posix_spawn(&pid, command, &fa, &attr, (char * const*)spawn_argv, env)) { *pidptr = pid; myp_add_locked(pid); - debug(D_CHILDS, "Spawned command: '%s' on pid %d from parent pid %d.", command, pid, getpid()); + debug(D_CHILDS, "Spawned command: \"%s\" on pid %d from parent pid %d.", command_to_be_logged, pid, getpid()); } else { myp_add_unlock(); - error("Failed to spawn command: '%s' from parent pid %d.", command, getpid()); - if (flags & FLAG_CREATE_PIPE) { + error("Failed to spawn command: \"%s\" from parent pid %d.", command_to_be_logged, getpid()); + if (flags & POPEN_FLAG_CREATE_PIPE) { fclose(fp); } ret = -1; } - if (flags & FLAG_CREATE_PIPE) { + if (flags & POPEN_FLAG_CREATE_PIPE) { close(pipefd[PIPE_WRITE]); if (0 == ret) // on success set FILE * pointer - *fpp = fp; + if(fpp) *fpp = fp; } if (!error) { @@ -181,8 +187,9 @@ static inline int custom_popene(const char *command, volatile pid_t *pidptr, cha error_after_posix_spawn_file_actions_init: if (posix_spawn_file_actions_destroy(&fa)) error("posix_spawn_file_actions_destroy"); + error_after_pipe: - if (flags & FLAG_CREATE_PIPE) { + if (flags & POPEN_FLAG_CREATE_PIPE) { if (fp) fclose(fp); else @@ -193,6 +200,41 @@ error_after_pipe: return -1; } +int custom_popene_variadic_internal_dont_use_directly(volatile pid_t *pidptr, char **env, uint8_t flags, FILE **fpp, const char *command, ...) { + // convert the variable list arguments into what posix_spawn() needs + // all arguments are expected strings + va_list args; + int args_count; + + // count the number variable parameters + // the variable parameters are expected NULL terminated + { + const char *s; + + va_start(args, command); + args_count = 0; + while ((s = va_arg(args, const char *))) args_count++; + va_end(args); + } + + // create a string pointer array as needed by posix_spawn() + // variable array in the stack + const char *spawn_argv[args_count + 1]; + { + const char *s; + va_start(args, command); + int i; + for (i = 0; i < args_count; i++) { + s = va_arg(args, const char *); + spawn_argv[i] = s; + } + spawn_argv[args_count] = NULL; + va_end(args); + } + + return custom_popene(pidptr, env, flags, fpp, command, spawn_argv); +} + // See man environ extern char **environ; @@ -252,19 +294,37 @@ int myp_reap(pid_t pid) { FILE *mypopen(const char *command, volatile pid_t *pidptr) { FILE *fp = NULL; - (void)custom_popene(command, pidptr, environ, FLAG_CREATE_PIPE | FLAG_CLOSE_FD, &fp); + const char *spawn_argv[] = { + "sh", + "-c", + command, + NULL + }; + (void)custom_popene(pidptr, environ, POPEN_FLAG_CREATE_PIPE|POPEN_FLAG_CLOSE_FD, &fp, "/bin/sh", spawn_argv); return fp; } FILE *mypopene(const char *command, volatile pid_t *pidptr, char **env) { FILE *fp = NULL; - (void)custom_popene(command, pidptr, env, FLAG_CREATE_PIPE | FLAG_CLOSE_FD, &fp); + const char *spawn_argv[] = { + "sh", + "-c", + command, + NULL + }; + (void)custom_popene( pidptr, env, POPEN_FLAG_CREATE_PIPE|POPEN_FLAG_CLOSE_FD, &fp, "/bin/sh", spawn_argv); return fp; } // returns 0 on success, -1 on failure int netdata_spawn(const char *command, volatile pid_t *pidptr) { - return custom_popene(command, pidptr, environ, 0, NULL); + const char *spawn_argv[] = { + "sh", + "-c", + command, + NULL + }; + return custom_popene( pidptr, environ, POPEN_FLAG_NONE, NULL, "/bin/sh", spawn_argv); } int custom_pclose(FILE *fp, pid_t pid) { diff --git a/libnetdata/popen/popen.h b/libnetdata/popen/popen.h index f387cff0a1..57eb9131e0 100644 --- a/libnetdata/popen/popen.h +++ b/libnetdata/popen/popen.h @@ -8,6 +8,23 @@ #define PIPE_READ 0 #define PIPE_WRITE 1 +/* custom_popene_variadic_internal_dont_use_directly flag definitions */ +#define POPEN_FLAG_NONE 0 +#define POPEN_FLAG_CREATE_PIPE 1 // Create a pipe like popen() when set, otherwise set stdout to /dev/null +#define POPEN_FLAG_CLOSE_FD 2 // Close all file descriptors other than STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO + +// the flags to be used by default +#define POPEN_FLAGS_DEFAULT (POPEN_FLAG_CREATE_PIPE|POPEN_FLAG_CLOSE_FD) + +// mypopen_raw is the interface to use instead of custom_popene_variadic_internal_dont_use_directly() +// mypopen_raw will add the terminating NULL at the arguments list +// we append the parameter 'command' twice - this is because the underlying call needs the command to execute and the argv[0] to pass to it +#define mypopen_raw_default_flags_and_environment(pidptr, fpp, command, args...) custom_popene_variadic_internal_dont_use_directly(pidptr, environ, POPEN_FLAGS_DEFAULT, fpp, command, command, ##args, NULL) +#define mypopen_raw_default_flags(pidptr, env, fpp, command, args...) custom_popene_variadic_internal_dont_use_directly(pidptr, env, POPEN_FLAGS_DEFAULT, fpp, command, command, ##args, NULL) +#define mypopen_raw(pidptr, env, flags, fpp, command, args...) custom_popene_variadic_internal_dont_use_directly(pidptr, env, flags, fpp, command, command, ##args, NULL) + +extern int custom_popene_variadic_internal_dont_use_directly(volatile pid_t *pidptr, char **env, uint8_t flags, FILE **fpp, const char *command, ...); + extern FILE *mypopen(const char *command, volatile pid_t *pidptr); extern FILE *mypopene(const char *command, volatile pid_t *pidptr, char **env); extern int mypclose(FILE *fp, pid_t pid); |