From 5d70db506c06955f45e1ce7791c902097fb15187da2b98c3fa7f2c64f76dcca5 Mon Sep 17 00:00:00 2001 From: Matthias Bach Date: Tue, 23 Jul 2019 19:48:05 +0000 Subject: [PATCH] Accepting request 717879 from home:priand:branches:games:tools - Update to version 1.4 * Add new D-Bus methods/properties for use by external tools such as the GameMode GNOME Shell extension. * Fix I/O priority and niceness optimisations to apply to the whole process rather than just the thread that requests GameMode. * gamemoded will now automatically reload the configuration file when it is changed and update optimisations on current clients. * Add support for using the client library inside Flatpak by communicating with the daemon via a portal. * Client library now uses libdbus rather than sd-bus. * Fix gamemoderun to use the correct library path depending on whether the app is 32-bit or 64-bit. * Support the GAMEMODERUNEXEC environment variable to specify an extra wrapper command for games launched with gamemoderun (e.g. a hybrid GPU wrapper such as optirun). * Various other fixes and improvements. - Removed gpuctl-fixes.patch already included in 1.4 - Added "BuildRequires: pkgconfig(dbus-1)" to spec file which is needed by version 1.4 OBS-URL: https://build.opensuse.org/request/show/717879 OBS-URL: https://build.opensuse.org/package/show/games:tools/gamemode?expand=0&rev=15 --- _service | 2 +- _servicedata | 2 +- gamemode-1.3.1.tar.xz | 3 - gamemode-1.4.tar.xz | 3 + gamemode.changes | 27 ++ gamemode.spec | 6 +- gpuctl-fixes.patch | 644 ------------------------------------------ 7 files changed, 35 insertions(+), 652 deletions(-) delete mode 100644 gamemode-1.3.1.tar.xz create mode 100644 gamemode-1.4.tar.xz delete mode 100644 gpuctl-fixes.patch diff --git a/_service b/_service index 4e9a0a0..c681051 100644 --- a/_service +++ b/_service @@ -2,7 +2,7 @@ https://github.com/FeralInteractive/gamemode.git git - 1.3.1 + 1.4 @PARENT_TAG@ enable diff --git a/_servicedata b/_servicedata index a7eaa35..afb9581 100644 --- a/_servicedata +++ b/_servicedata @@ -1,4 +1,4 @@ https://github.com/FeralInteractive/gamemode.git - bc20bef283fb947d7cee23306860af04f723ae9f + 7ced21f4b30cc4ceb67928889bc57af8784f1009 diff --git a/gamemode-1.3.1.tar.xz b/gamemode-1.3.1.tar.xz deleted file mode 100644 index 7ed7118..0000000 --- a/gamemode-1.3.1.tar.xz +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:b84ed18d7df4a35ba5a73659b5ee9504c3361e04c1d23ba78bf6507d11224fa4 -size 55908 diff --git a/gamemode-1.4.tar.xz b/gamemode-1.4.tar.xz new file mode 100644 index 0000000..4676c52 --- /dev/null +++ b/gamemode-1.4.tar.xz @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:e83f07dc91bbd27d1faadd69f7fda899810001e2a256be8061e36dee0e858f4a +size 64756 diff --git a/gamemode.changes b/gamemode.changes index 2ff12b5..4663dbe 100644 --- a/gamemode.changes +++ b/gamemode.changes @@ -1,3 +1,30 @@ +------------------------------------------------------------------- +Tue Jul 23 11:19:07 UTC 2019 - Andreas Prittwitz + +- Update to version 1.4 + + * Add new D-Bus methods/properties for use by external tools + such as the GameMode GNOME Shell extension. + * Fix I/O priority and niceness optimisations to apply to the + whole process rather than just the thread that + requests GameMode. + * gamemoded will now automatically reload the configuration + file when it is changed and update optimisations on + current clients. + * Add support for using the client library inside Flatpak by + communicating with the daemon via a portal. + * Client library now uses libdbus rather than sd-bus. + * Fix gamemoderun to use the correct library path depending on + whether the app is 32-bit or 64-bit. + * Support the GAMEMODERUNEXEC environment variable to specify + an extra wrapper command for games launched with gamemoderun + (e.g. a hybrid GPU wrapper such as optirun). + * Various other fixes and improvements. + +- Removed gpuctl-fixes.patch already included in 1.4 +- Added "BuildRequires: pkgconfig(dbus-1)" to spec file which + is needed by version 1.4 + ------------------------------------------------------------------- Wed May 22 13:58:09 UTC 2019 - Christophe Giboudeaux diff --git a/gamemode.spec b/gamemode.spec index 56c7077..a85d11e 100644 --- a/gamemode.spec +++ b/gamemode.spec @@ -18,7 +18,7 @@ Name: gamemode -Version: 1.3.1 +Version: 1.4 Release: 0 Summary: Daemon/library combo for changing Linux system performance on demand License: BSD-3-Clause @@ -28,12 +28,12 @@ Source0: gamemode-%{version}.tar.xz Source1: gamemode-rpmlintrc Source2: README.openSUSE Source3: baselibs.conf -Patch1: gpuctl-fixes.patch BuildRequires: cmake BuildRequires: meson BuildRequires: ninja BuildRequires: pkgconfig BuildRequires: polkit-devel +BuildRequires: pkgconfig(dbus-1) # Yes, it needs both BuildRequires: pkgconfig(libsystemd) BuildRequires: pkgconfig(systemd) @@ -110,7 +110,7 @@ built-in GameMode support. %prep %setup -q -%patch1 -p1 + cp %{SOURCE2} . %build diff --git a/gpuctl-fixes.patch b/gpuctl-fixes.patch deleted file mode 100644 index 451f049..0000000 --- a/gpuctl-fixes.patch +++ /dev/null @@ -1,644 +0,0 @@ -From cbc6ce7e2677f9c3759423bfef5a03ff4ad7575c Mon Sep 17 00:00:00 2001 -From: Matthias Gerstner -Date: Wed, 3 Apr 2019 16:18:36 +0200 -Subject: [PATCH 1/4] gpuclockctl: fix return value of get_gpu_state_amd() - ---- - daemon/gpuclockctl.c | 2 +- - 1 file changed, 1 insertion(+), 1 deletion(-) - -diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c -index 73c17c5..8598a9f 100644 ---- a/daemon/gpuclockctl.c -+++ b/daemon/gpuclockctl.c -@@ -303,7 +303,7 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) - /* 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'; -- return info == NULL; -+ return 0; - } - - /* - -From fd576682a3720a22e12e63014ec9f01621c6ed0f Mon Sep 17 00:00:00 2001 -From: Matthias Gerstner -Date: Wed, 3 Apr 2019 16:27:12 +0200 -Subject: [PATCH 2/4] daemon: fix file descriptor leaks - ---- - daemon/daemon_config.c | 1 + - daemon/governors-query.c | 1 + - daemon/gpu-control.c | 5 ++++- - daemon/gpuclockctl.c | 26 +++++++++++++++++--------- - 4 files changed, 23 insertions(+), 10 deletions(-) - -diff --git a/daemon/daemon_config.c b/daemon/daemon_config.c -index 675c195..23e0e76 100644 ---- a/daemon/daemon_config.c -+++ b/daemon/daemon_config.c -@@ -362,6 +362,7 @@ static void load_config_files(GameModeConfig *self) - if (error) { - LOG_MSG("Failed to parse config file - error on line %d!\n", error); - } -+ fclose(f); - } - free(path); - } -diff --git a/daemon/governors-query.c b/daemon/governors-query.c -index 9e0af0f..9d483c0 100644 ---- a/daemon/governors-query.c -+++ b/daemon/governors-query.c -@@ -133,6 +133,7 @@ const char *get_gov_state(void) - /* Don't handle the mixed case, this shouldn't ever happen - * But it is a clear sign we shouldn't carry on */ - LOG_ERROR("Governors malformed: got \"%s\", expected \"%s\"", contents, governor); -+ fclose(f); - return "malformed"; - } - -diff --git a/daemon/gpu-control.c b/daemon/gpu-control.c -index 147cdd1..a2699c0 100644 ---- a/daemon/gpu-control.c -+++ b/daemon/gpu-control.c -@@ -50,7 +50,10 @@ enum GPUVendor gamemode_get_gpu_vendor(long device) - return Vendor_Invalid; - } - char buff[64]; -- if (fgets(buff, 64, file) != NULL) { -+ bool got_line = fgets(buff, 64, file) != NULL; -+ fclose(file); -+ -+ if (got_line) { - vendor = strtol(buff, NULL, 0); - } else { - LOG_ERROR("Coudn't read contents of file %s, will not apply optimisations!\n", path); -diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c -index 8598a9f..873ab54 100644 ---- a/daemon/gpuclockctl.c -+++ b/daemon/gpuclockctl.c -@@ -289,21 +289,27 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) - return -1; - } - -+ int ret = 0; -+ - char buff[CONFIG_VALUE_MAX] = { 0 }; - if (!fgets(buff, CONFIG_VALUE_MAX, file)) { - LOG_ERROR("Could not read file %s (%s)!\n", path, strerror(errno)); -- return -1; -+ ret = -1; - } - - if (fclose(file) != 0) { - LOG_ERROR("Could not close %s after reading (%s)!\n", path, strerror(errno)); -- return -1; -+ ret = -1; - } - -- /* 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'; -- return 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'; -+ } -+ -+ return ret; - } - - /* -@@ -320,17 +326,19 @@ static int set_gpu_state_amd_file(const char *filename, long device, const char - return -1; - } - -+ int ret = 0; -+ - if (fprintf(file, "%s", value) < 0) { - LOG_ERROR("Could not write to %s (%s)!\n", path, strerror(errno)); -- return -1; -+ ret = -1; - } - - if (fclose(file) != 0) { - LOG_ERROR("Could not close %s after writing (%s)!\n", path, strerror(errno)); -- return -1; -+ ret = -1; - } - -- return 0; -+ return ret; - } - - /** - -From 54212fa81d80b7a6981faed09777e55ce8e86963 Mon Sep 17 00:00:00 2001 -From: Matthias Gerstner -Date: Wed, 3 Apr 2019 16:46:38 +0200 -Subject: [PATCH 3/4] gpuclockctl: refactor buffer size specification and avoid - redundancies - ---- - daemon/gpuclockctl.c | 116 ++++++++++++++++++++++++------------------- - 1 file changed, 64 insertions(+), 52 deletions(-) - -diff --git a/daemon/gpuclockctl.c b/daemon/gpuclockctl.c -index 873ab54..cac49bd 100644 ---- a/daemon/gpuclockctl.c -+++ b/daemon/gpuclockctl.c -@@ -36,6 +36,8 @@ POSSIBILITY OF SUCH DAMAGE. - #include "external-helper.h" - #include "gpu-control.h" - -+#include -+ - /* NV constants */ - #define NV_CORE_OFFSET_ATTRIBUTE "GPUGraphicsClockOffset" - #define NV_MEM_OFFSET_ATTRIBUTE "GPUMemoryTransferRateOffset" -@@ -44,6 +46,7 @@ POSSIBILITY OF SUCH DAMAGE. - #define NV_PCIDEVICE_ATTRIBUTE "PCIDevice" - #define NV_ATTRIBUTE_FORMAT "[gpu:%ld]/%s" - #define NV_PERF_LEVEL_FORMAT "[%ld]" -+#define NV_ARG_MAX 128 - - /* AMD constants */ - #define AMD_DRM_PATH "/sys/class/drm/card%ld/device/%s" -@@ -64,6 +67,30 @@ static void print_usage_and_exit(void) - exit(EXIT_FAILURE); - } - -+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 }; -+ if (run_external_process(exec_args, out, -1) != 0) { -+ LOG_ERROR("Failed to get %s!\n", attr); -+ out[0] = 0; -+ return NULL; -+ } -+ -+ return &out[0]; -+} -+ -+static int set_nv_attr(const char *attr) -+{ -+ const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", attr, NULL }; -+ if (run_external_process(exec_args_core, NULL, -1) != 0) { -+ LOG_ERROR("Failed to set %s!\n", attr); -+ return -1; -+ } -+ -+ return 0; -+} -+ - /* Get the nvidia driver index for the current GPU */ - static long get_gpu_index_id_nv(struct GameModeGPUInfo *info) - { -@@ -108,23 +135,21 @@ static long get_max_perf_level_nv(struct GameModeGPUInfo *info) - if (!getenv("DISPLAY")) - LOG_ERROR("Getting Nvidia parameters requires DISPLAY to be set - will likely fail!\n"); - -- char arg[128] = { 0 }; -- char buf[EXTERNAL_BUFFER_MAX] = { 0 }; -+ char arg[NV_ARG_MAX] = { 0 }; -+ const char *attr; - -- snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE); -- const char *exec_args[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; -- if (run_external_process(exec_args, buf, -1) != 0) { -- LOG_ERROR("Failed to get %s!\n", arg); -+ snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_PERFMODES_ATTRIBUTE); -+ if ((attr = get_nv_attr(arg)) == NULL) { - return -1; - } - -- char *ptr = strrchr(buf, ';'); -+ char *ptr = strrchr(attr, ';'); - long level = -1; - if (!ptr || sscanf(ptr, "; perf=%ld", &level) != 1) { - LOG_ERROR( - "Output didn't match expected format, couldn't discern highest perf level from " - "nvidia-settings!\n"); -- LOG_ERROR("Output:%s\n", buf); -+ LOG_ERROR("Output:%s\n", attr); - return -1; - } - -@@ -142,59 +167,53 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info) - - long perf_level = get_max_perf_level_nv(info); - -- char arg[128] = { 0 }; -- char buf[EXTERNAL_BUFFER_MAX] = { 0 }; -+ char arg[NV_ARG_MAX] = { 0 }; -+ const char *attr; - char *end; - - /* Get the GPUGraphicsClockOffset parameter */ - snprintf(arg, -- 128, -+ NV_ARG_MAX, - NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT, - info->device, - NV_CORE_OFFSET_ATTRIBUTE, - perf_level); -- const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; -- if (run_external_process(exec_args_core, buf, -1) != 0) { -- LOG_ERROR("Failed to get %s!\n", arg); -+ if ((attr = get_nv_attr(arg)) == NULL) { - return -1; - } - -- info->nv_core = strtol(buf, &end, 10); -- if (end == buf) { -- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); -+ info->nv_core = strtol(attr, &end, 10); -+ if (end == attr) { -+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); - return -1; - } - - /* Get the GPUMemoryTransferRateOffset parameter */ - snprintf(arg, -- 128, -+ NV_ARG_MAX, - NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT, - info->device, - NV_MEM_OFFSET_ATTRIBUTE, - perf_level); -- const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; -- if (run_external_process(exec_args_mem, buf, -1) != 0) { -- LOG_ERROR("Failed to get %s!\n", arg); -+ if ((attr = get_nv_attr(arg)) == NULL) { - return -1; - } - -- info->nv_mem = strtol(buf, &end, 10); -- if (end == buf) { -- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); -+ info->nv_mem = strtol(attr, &end, 10); -+ if (end == attr) { -+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); - return -1; - } - - /* Get the GPUPowerMizerMode parameter */ -- snprintf(arg, 128, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE); -- const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL }; -- if (run_external_process(exec_args_pm, buf, -1) != 0) { -- LOG_ERROR("Failed to get %s!\n", arg); -+ snprintf(arg, NV_ARG_MAX, NV_ATTRIBUTE_FORMAT, info->device, NV_POWERMIZER_MODE_ATTRIBUTE); -+ if ((attr = get_nv_attr(arg)) == NULL) { - return -1; - } - -- info->nv_powermizer_mode = strtol(buf, &end, 10); -- if (end == buf) { -- LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf); -+ info->nv_powermizer_mode = strtol(attr, &end, 10); -+ if (end == attr) { -+ LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, attr); - return -1; - } - -@@ -218,20 +237,18 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) - - long perf_level = get_max_perf_level_nv(info); - -- char arg[128] = { 0 }; -+ char arg[NV_ARG_MAX] = { 0 }; - - /* Set the GPUGraphicsClockOffset parameter */ - if (info->nv_core != -1) { - snprintf(arg, -- 128, -+ NV_ARG_MAX, - NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld", - info->device, - NV_CORE_OFFSET_ATTRIBUTE, - perf_level, - info->nv_core); -- const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; -- if (run_external_process(exec_args_core, NULL, -1) != 0) { -- LOG_ERROR("Failed to set %s!\n", arg); -+ if (set_nv_attr(arg) != 0) { - status = -1; - } - } -@@ -239,15 +256,13 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) - /* Set the GPUMemoryTransferRateOffset parameter */ - if (info->nv_mem != -1) { - snprintf(arg, -- 128, -+ NV_ARG_MAX, - NV_ATTRIBUTE_FORMAT NV_PERF_LEVEL_FORMAT "=%ld", - info->device, - NV_MEM_OFFSET_ATTRIBUTE, - perf_level, - info->nv_mem); -- const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; -- if (run_external_process(exec_args_mem, NULL, -1) != 0) { -- LOG_ERROR("Failed to set %s!\n", arg); -+ if (set_nv_attr(arg) != 0) { - status = -1; - } - } -@@ -255,14 +270,12 @@ static int set_gpu_state_nv(struct GameModeGPUInfo *info) - /* Set the GPUPowerMizerMode parameter if requested */ - if (info->nv_powermizer_mode != -1) { - snprintf(arg, -- 128, -+ NV_ARG_MAX, - NV_ATTRIBUTE_FORMAT "=%ld", - info->device, - NV_POWERMIZER_MODE_ATTRIBUTE, - info->nv_powermizer_mode); -- const char *exec_args_pm[] = { "/usr/bin/nvidia-settings", "-a", arg, NULL }; -- if (run_external_process(exec_args_pm, NULL, -1) != 0) { -- LOG_ERROR("Failed to set %s!\n", arg); -+ if (set_nv_attr(arg) != 0) { - status = -1; - } - } -@@ -280,8 +293,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) - return -1; - - /* Get the contents of power_dpm_force_performance_level */ -- char path[64]; -- snprintf(path, 64, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level"); -+ char path[PATH_MAX]; -+ snprintf(path, PATH_MAX, AMD_DRM_PATH, info->device, "power_dpm_force_performance_level"); - - FILE *file = fopen(path, "r"); - if (!file) { -@@ -317,8 +330,8 @@ static int get_gpu_state_amd(struct GameModeGPUInfo *info) - */ - static int set_gpu_state_amd_file(const char *filename, long device, const char *value) - { -- char path[64]; -- snprintf(path, 64, AMD_DRM_PATH, device, filename); -+ char path[PATH_MAX]; -+ snprintf(path, PATH_MAX, AMD_DRM_PATH, device, filename); - - FILE *file = fopen(path, "w"); - if (!file) { -@@ -398,10 +411,11 @@ static long get_generic_value(const char *val) - */ - int main(int argc, char *argv[]) - { -+ struct GameModeGPUInfo info; -+ memset(&info, 0, sizeof(info)); -+ - if (argc == 3 && strncmp(argv[2], "get", 3) == 0) { - /* Get and verify the vendor and device */ -- struct GameModeGPUInfo info; -- memset(&info, 0, sizeof(info)); - info.device = get_device(argv[1]); - info.vendor = gamemode_get_gpu_vendor(info.device); - -@@ -428,8 +442,6 @@ int main(int argc, char *argv[]) - - } else if (argc >= 4 && argc <= 7 && strncmp(argv[2], "set", 3) == 0) { - /* Get and verify the vendor and device */ -- struct GameModeGPUInfo info; -- memset(&info, 0, sizeof(info)); - info.device = get_device(argv[1]); - info.vendor = gamemode_get_gpu_vendor(info.device); - - -From 9320a94638bcc216104ffe822299def89eaf8945 Mon Sep 17 00:00:00 2001 -From: Matthias Gerstner -Date: Thu, 4 Apr 2019 11:39:17 +0200 -Subject: [PATCH 4/4] external-helper: improve run_external_process() - robustness - -run_external_process() contained pipe file descriptors leaks (e.g. one -pipe end was never closed). Also the stdout might have been captured -incomplete, since only a single read() was performed on the pipe. -Furthermore should a child process try to write a larger amount of data -onto the pipe then it will become stuck, because the parent process -isn't consuming the data. Thus the timeout would trigger in these cases -although the child process does nothing wrong. - -This commit changes the implementation to follow a select() based -approach that continually reads from the pipe, but discards data that -doesn't fit in the provided buffer. ---- - daemon/external-helper.c | 136 ++++++++++++++++++++++++++------------- - daemon/gpuclockctl.c | 5 +- - 2 files changed, 93 insertions(+), 48 deletions(-) - -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,39 +148,31 @@ 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]); -+ -+ 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); - } - -- internal[output_size] = 0; -- - if (waitpid(p, &status, 0) < 0) { - LOG_ERROR("Failed to waitpid(%d): %s\n", (int)p, strerror(errno)); - return -1; -@@ -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';