Add run_external_process_get_output function to get output as well

daemon/external-helper.c

@@ -31,6 +31,7 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #define _GNU_SOURCE
 
+#include "external-helper.h"
 #include "logging.h"
 
 #include <linux/limits.h>
@@ -45,8 +46,6 @@ int run_external_process(const char *const *exec_args)
 {
 	pid_t p;
 	int status = 0;
-	int ret = 0;
-	int r = -1;
 
 	if ((p = fork()) < 0) {
 		LOG_ERROR("Failed to fork(): %s\n", strerror(errno));
@@ -59,23 +58,74 @@ int run_external_process(const char *const *exec_args)
 		 *   bindings that these objects are completely constant.
 		 * http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
 		 */
-		if ((r = execv(exec_args[0], (char *const *)exec_args)) != 0) {
+		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);
-	} else {
-		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]);
-		}
 	}
 
-	if ((ret = WEXITSTATUS(status)) != 0) {
+	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) {
+		LOG_ERROR("Failed to read from process %s: %s\n", exec_args[0], strerror(errno));
+		return -1;
+	}
+
+	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;
 	}

daemon/external-helper.h

@@ -31,5 +31,10 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #pragma once
 
+#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]);

daemon/gpuclockctl.c

@@ -64,9 +64,9 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	if (info->vendor != Vendor_NVIDIA)
 		return -1;
 
-	char cmd[128] = { 0 };
 	char arg[128] = { 0 };
-	char buf[128] = { 0 };
+	char buf[EXTERNAL_BUFFER_MAX] = { 0 };
+	char *end;
 
 	/* Set the GPUGraphicsClockOffset parameter */
 	snprintf(arg,
@@ -75,28 +75,15 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	         info->device,
 	         NV_CORE_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level);
-	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
+	const char *exec_args_core[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
-
+	if (run_external_process_get_output(exec_args_core, buf) != 0) {
-	FILE* run = popen(cmd, "r");
+		LOG_ERROR("Failed to set %s!\n", arg);
-	if( run == NULL)
-	{
-		LOG_ERROR("Command [%s] failed to run!\n", cmd);
 		return -1;
 	}
 
-	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
-	{
-		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
-		return -1;
-	}
-
-	char* end = NULL;
 	info->core = strtol(buf, &end, 10);
-	pclose(run);
+	if (end == buf) {
-
+		LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
-	if( end == buf )
-	{
-		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
 		return -1;
 	}
 
@@ -107,28 +94,15 @@ static int get_gpu_state_nv(struct GameModeGPUInfo *info)
 	         info->device,
 	         NV_MEM_OFFSET_ATTRIBUTE,
 	         info->nv_perf_level);
-	snprintf(cmd, 128, "/usr/bin/nvidia-settings -q %s -t", arg );
+	const char *exec_args_mem[] = { "/usr/bin/nvidia-settings", "-q", arg, "-t", NULL };
-
+	if (run_external_process_get_output(exec_args_mem, buf) != 0) {
-	run = popen(cmd, "r");
+		LOG_ERROR("Failed to set %s!\n", arg);
-	if( run == NULL)
-	{
-		LOG_ERROR("Command [%s] failed to run!\n", cmd);
 		return -1;
 	}
 
-	if( fgets( buf, sizeof(buf)-1, run) == NULL && buf[0] != '\0' )
-	{
-		LOG_ERROR("Failed to get output of [%s]!\n",cmd);
-		return -1;
-	}
-
-	end = NULL;
 	info->mem = strtol(buf, &end, 10);
-	pclose(run);
+	if (end == buf) {
-
+		LOG_ERROR("Failed to parse output for \"%s\" output was \"%s\"!\n", arg, buf);
-	if( end == buf )
-	{
-		LOG_ERROR("Failed to parse output of [%s] output [%s]!\n",cmd, buf);
 		return -1;
 	}
 
@@ -290,11 +264,11 @@ int main(int argc, char *argv[])
 		switch (info.vendor) {
 		case Vendor_NVIDIA:
 			/* Get nvidia power level */
-			if( get_gpu_state_nv(&info) != 0 )
+			if (get_gpu_state_nv(&info) != 0)
 				exit(EXIT_FAILURE);
 			break;
 		case Vendor_AMD:
-			if( get_gpu_state_amd(&info) != 0)
+			if (get_gpu_state_amd(&info) != 0)
 				exit(EXIT_FAILURE);
 			break;
 		default: