From 4578af47baa7ebdc9de06896288b209fd63427e2 Mon Sep 17 00:00:00 2001 From: Marc Di Luzio Date: Thu, 21 Feb 2019 20:03:59 +0000 Subject: [PATCH] Combine the two run_external_process functions so they both have the same timeout protection --- daemon/external-helper.c | 60 ++++++++-------------------------------- daemon/external-helper.h | 5 +--- daemon/gamemode-gpu.c | 4 +-- daemon/gamemode-tests.c | 4 +-- daemon/gamemode.c | 6 ++-- daemon/gpuclockctl.c | 8 +++--- 6 files changed, 24 insertions(+), 63 deletions(-) diff --git a/daemon/external-helper.c b/daemon/external-helper.c index 18335d8..fcdcaa4 100644 --- a/daemon/external-helper.c +++ b/daemon/external-helper.c @@ -43,10 +43,16 @@ POSSIBILITY OF SUCH DAMAGE. /** * Call an external process */ -int run_external_process(const char *const *exec_args) +int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]) { pid_t p; int status = 0; + int pipes[2]; + + if (pipe(pipes) == -1) { + LOG_ERROR("Could not create pipe: %s!\n", strerror(errno)); + return -1; + } /* set up our signaling for the child and the timout */ sigset_t mask; @@ -62,6 +68,10 @@ int run_external_process(const char *const *exec_args) LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); return false; } else if (p == 0) { + /* Send STDOUT to the pipe */ + dup2(pipes[1], STDOUT_FILENO); + close(pipes[0]); + close(pipes[1]); /* Execute the command */ /* Note about cast: * The statement about argv[] and envp[] being constants is @@ -97,54 +107,8 @@ int run_external_process(const char *const *exec_args) break; } - if (waitpid(p, &status, 0) < 0) { - LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno)); - return -1; - } - - /* i.e. sigsev */ - if (!WIFEXITED(status)) { - LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]); - } else if (WEXITSTATUS(status) != 0) { - LOG_ERROR("External process failed\n"); - return -1; - } - - return 0; -} - -/** - * Call an external process and get output - */ -int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]) -{ - pid_t p; - int status = 0; - int pipes[2]; - - if (pipe(pipes) == -1) { - LOG_ERROR("Could not create pipe: %s!\n", strerror(errno)); - return -1; - } - - if ((p = fork()) < 0) { - LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); - return false; - } else if (p == 0) { - /* Send STDOUT to the pipe */ - dup2(pipes[1], STDOUT_FILENO); - close(pipes[0]); - close(pipes[1]); - /* Launch the process */ - if (execv(exec_args[0], (char *const *)exec_args) != 0) { - LOG_ERROR("Failed to execute external process %s: %s\n", exec_args[0], strerror(errno)); - exit(EXIT_FAILURE); - } - _exit(EXIT_SUCCESS); - } - close(pipes[1]); - if (read(pipes[0], buffer, EXTERNAL_BUFFER_MAX) < 0) { + if (buffer && read(pipes[0], buffer, EXTERNAL_BUFFER_MAX) < 0) { LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); return -1; } diff --git a/daemon/external-helper.h b/daemon/external-helper.h index 0febe20..73d0a01 100644 --- a/daemon/external-helper.h +++ b/daemon/external-helper.h @@ -34,7 +34,4 @@ POSSIBILITY OF SUCH DAMAGE. #define EXTERNAL_BUFFER_MAX 1024 /* Run an external process and capture the return value */ -int run_external_process(const char *const *exec_args); - -/* Run an external process and capture the return value as well as output */ -int run_external_process_get_output(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]); +int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFFER_MAX]); diff --git a/daemon/gamemode-gpu.c b/daemon/gamemode-gpu.c index 2ae3374..9c5de33 100644 --- a/daemon/gamemode-gpu.c +++ b/daemon/gamemode-gpu.c @@ -229,7 +229,7 @@ int game_mode_apply_gpu(const GameModeGPUInfo *info, bool apply) NULL, }; - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL) != 0) { LOG_ERROR("Failed to call gpuclockctl, could not apply optimisations!\n"); return -1; } @@ -262,7 +262,7 @@ int game_mode_get_gpu(GameModeGPUInfo *info) }; char buffer[EXTERNAL_BUFFER_MAX] = { 0 }; - if (run_external_process_get_output(exec_args, buffer) != 0) { + if (run_external_process(exec_args, buffer) != 0) { LOG_ERROR("Failed to call gpuclockctl, could get values!\n"); return -1; } diff --git a/daemon/gamemode-tests.c b/daemon/gamemode-tests.c index 2df541e..a65cbb2 100644 --- a/daemon/gamemode-tests.c +++ b/daemon/gamemode-tests.c @@ -372,7 +372,7 @@ static int run_custom_scripts_tests(struct GameModeConfig *config) LOG_MSG(":::: Running start script [%s]\n", startscripts[i]); const char *args[] = { "/bin/sh", "-c", startscripts[i], NULL }; - int ret = run_external_process(args); + int ret = run_external_process(args, NULL); if (ret == 0) LOG_MSG(":::: Passed\n"); @@ -395,7 +395,7 @@ static int run_custom_scripts_tests(struct GameModeConfig *config) LOG_MSG(":::: Running end script [%s]\n", endscripts[i]); const char *args[] = { "/bin/sh", "-c", endscripts[i], NULL }; - int ret = run_external_process(args); + int ret = run_external_process(args, NULL); if (ret == 0) LOG_MSG(":::: Passed\n"); diff --git a/daemon/gamemode.c b/daemon/gamemode.c index f445374..78fee5d 100644 --- a/daemon/gamemode.c +++ b/daemon/gamemode.c @@ -189,7 +189,7 @@ static void game_mode_context_enter(GameModeContext *self) }; LOG_MSG("Requesting update of governor policy to %s\n", desiredGov); - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL) != 0) { LOG_ERROR("Failed to update cpu governor policy\n"); /* if the set fails, clear the initial mode so we don't try and reset it back and fail * again, presumably */ @@ -242,7 +242,7 @@ static void game_mode_context_leave(GameModeContext *self) }; LOG_MSG("Requesting update of governor policy to %s\n", gov_mode); - if (run_external_process(exec_args) != 0) { + if (run_external_process(exec_args, NULL) != 0) { LOG_ERROR("Failed to update cpu governor policy\n"); } @@ -693,7 +693,7 @@ static void game_mode_execute_scripts(char scripts[CONFIG_LIST_MAX][CONFIG_VALUE LOG_MSG("Executing script [%s]\n", scripts[i]); int err; const char *args[] = { "/bin/sh", "-c", scripts[i], NULL }; - if ((err = run_external_process(args)) != 0) { + if ((err = run_external_process(args, NULL)) != 0) { /* Log the failure, but this is not fatal */ LOG_ERROR("Script [%s] failed with error %d\n", scripts[i], err); } diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c index 28963f4..26e1d0a 100644 --- a/daemon/gpuclockctl.c +++ b/daemon/gpuclockctl.c @@ -79,7 +79,7 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) NV_CORE_OFFSET_ATTRIBUTE, info->nv_perf_level); const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process_get_output(exec_args_core, buf) != 0) { + if (run_external_process(exec_args_core, buf) != 0) { LOG_ERROR("Failed to set %s!\n", arg); return -1; } @@ -98,7 +98,7 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) NV_MEM_OFFSET_ATTRIBUTE, info->nv_perf_level); const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; - if (run_external_process_get_output(exec_args_mem, buf) != 0) { + if (run_external_process(exec_args_mem, buf) != 0) { LOG_ERROR("Failed to set %s!\n", arg); return -1; } @@ -145,7 +145,7 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) info->nv_perf_level, info->core); const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", core_arg, NULL }; - if (run_external_process(exec_args_core) != 0) { + if (run_external_process(exec_args_core, NULL) != 0) { LOG_ERROR("Failed to set %s!\n", core_arg); return -1; } @@ -160,7 +160,7 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) info->nv_perf_level, info->mem); const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", mem_arg, NULL }; - if (run_external_process(exec_args_mem) != 0) { + if (run_external_process(exec_args_mem, NULL) != 0) { LOG_ERROR("Failed to set %s!\n", mem_arg); return -1; }