Combine the two run_external_process functions so they both have the same timeout protection

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;
 	}

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]);

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;
 	}

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");

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);
 		}

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;
 	}