diff --git a/daemon/external-helper.c b/daemon/external-helper.c index 52361f9..62cf29a 100644 --- a/daemon/external-helper.c +++ b/daemon/external-helper.c @@ -36,11 +36,75 @@ POSSIBILITY OF SUCH DAMAGE. #include #include +#include +#include #include -#include #include -static const int default_timeout = 5; +static const int DEFAULT_TIMEOUT = 5; + +static int read_child_stdout(int pipe_fd, char buffer[EXTERNAL_BUFFER_MAX], int tsec) +{ + fd_set fds; + struct timeval timeout; + int num_readable = 0; + ssize_t buffer_bytes_read = 0; + ssize_t just_read = 0; + bool buffer_full = false; + char discard_buffer[EXTERNAL_BUFFER_MAX]; + + /* Set up the timout */ + timeout.tv_sec = tsec; + timeout.tv_usec = 0; + + FD_ZERO(&fds); + + /* Wait for the child to finish up with a timout */ + while (true) { + FD_SET(pipe_fd, &fds); + num_readable = select(pipe_fd + 1, &fds, NULL, NULL, &timeout); + + if (num_readable < 0) { + if (errno == EINTR) { + continue; + } else { + LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno)); + return -1; + } + } else if (num_readable == 0) { + return -2; + } + + if (!buffer_full) { + just_read = read(pipe_fd, + buffer + buffer_bytes_read, + EXTERNAL_BUFFER_MAX - (size_t)buffer_bytes_read - 1); + } else { + just_read = read(pipe_fd, discard_buffer, EXTERNAL_BUFFER_MAX - 1); + } + + if (just_read < 0) { + return -1; + } else if (just_read == 0) { + // EOF encountered + break; + } + + if (!buffer_full) { + buffer_bytes_read += just_read; + + if (buffer_bytes_read == EXTERNAL_BUFFER_MAX - 1) { + // our buffer is exhausted, discard the rest + // of the output + buffer_full = true; + } + } + } + + buffer[buffer_bytes_read] = 0; + + return 0; +} /** * Call an external process @@ -50,6 +114,7 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF pid_t p; int status = 0; int pipes[2]; + int ret = 0; char internal[EXTERNAL_BUFFER_MAX] = { 0 }; if (pipe(pipes) == -1) { @@ -59,20 +124,12 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF /* Set the default timeout */ if (tsec == -1) { - tsec = default_timeout; - } - - /* set up our signaling for the child and the timout */ - sigset_t mask; - sigset_t omask; - sigemptyset(&mask); - sigaddset(&mask, SIGCHLD); - if (sigprocmask(SIG_BLOCK, &mask, &omask) < 0) { - LOG_ERROR("sigprocmask failed: %s\n", strerror(errno)); - return -1; + tsec = DEFAULT_TIMEOUT; } if ((p = fork()) < 0) { + close(pipes[0]); + close(pipes[1]); LOG_ERROR("Failed to fork(): %s\n", strerror(errno)); return false; } else if (p == 0) { @@ -91,38 +148,30 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF LOG_ERROR("Failed to execute external process: %s %s\n", exec_args[0], strerror(errno)); exit(EXIT_FAILURE); } - _exit(EXIT_SUCCESS); - } - - /* Set up the timout */ - struct timespec timeout; - timeout.tv_sec = tsec; - timeout.tv_nsec = 0; - - /* Wait for the child to finish up with a timout */ - while (true) { - if (sigtimedwait(&mask, NULL, &timeout) < 0) { - if (errno == EINTR) { - continue; - } else if (errno == EAGAIN) { - LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]); - kill(p, SIGKILL); - } else { - LOG_ERROR("sigtimedwait failed: %s\n", strerror(errno)); - return -1; - } - } - break; + // should never be reached + abort(); } + // close the write end of the pipe so we get signaled EOF once the + // child exits close(pipes[1]); - ssize_t output_size = read(pipes[0], internal, EXTERNAL_BUFFER_MAX - 1); - if (output_size < 0) { - LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); - return -1; - } + ret = read_child_stdout(pipes[0], internal, tsec); + close(pipes[0]); - internal[output_size] = 0; + if (ret != 0) { + if (ret == -2) { + LOG_ERROR("Child process timed out for %s, killing and returning\n", exec_args[0]); + kill(p, SIGKILL); + } else { + LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno)); + } + if (buffer) { + // make sure the buffer is a terminated empty string on error + buffer[0] = 0; + } + } else if (buffer) { + memcpy(buffer, internal, EXTERNAL_BUFFER_MAX); + } if (waitpid(p, &status, 0) < 0) { LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno)); @@ -134,12 +183,9 @@ int run_external_process(const char *const *exec_args, char buffer[EXTERNAL_BUFF LOG_ERROR("Child process '%s' exited abnormally\n", exec_args[0]); } else if (WEXITSTATUS(status) != 0) { LOG_ERROR("External process failed with exit code %d\n", WEXITSTATUS(status)); - LOG_ERROR("Output was: %s\n", buffer ? buffer : internal); + LOG_ERROR("Output was: %s\n", internal); return -1; } - if (buffer) - memcpy(buffer, internal, EXTERNAL_BUFFER_MAX); - return 0; } diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c index cac49bd..850bd05 100644 --- a/daemon/gpuclockctl.c +++ b/daemon/gpuclockctl.c @@ -67,7 +67,7 @@ static void print_usage_and_exit(void) exit(EXIT_FAILURE); } -static const char* get_nv_attr(const char *attr) +static const char *get_nv_attr(const char *attr) { static char out[EXTERNAL_BUFFER_MAX]; const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", attr, "-t", NULL }; @@ -315,8 +315,7 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) ret = -1; } - if( ret == 0 ) - { + if (ret == 0) { /* Copy in the value from the file */ strncpy(info->amd_performance_level, buff, CONFIG_VALUE_MAX - 1); info->amd_performance_level[CONFIG_VALUE_MAX - 1] = '\0';