From 8079fea60fba4f4136e0c45f1a26dc31d344b94b Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Wed, 20 Mar 2019 13:24:52 +0200 Subject: runner/executor: refactor error handling * Refactor to use goto error handling * Make execute_test_process noreturn to remove uninitialized variable warning * Check fork() return value Signed-off-by: Simon Ser Reviewed-by: Petri Latvala --- runner/executor.c | 68 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 35 insertions(+), 33 deletions(-) (limited to 'runner') diff --git a/runner/executor.c b/runner/executor.c index d535c276..791af4ae 100644 --- a/runner/executor.c +++ b/runner/executor.c @@ -753,9 +753,10 @@ static int monitor_output(pid_t child, return killed; } -static void execute_test_process(int outfd, int errfd, - struct settings *settings, - struct job_list_entry *entry) +static void __attribute__((noreturn)) +execute_test_process(int outfd, int errfd, + struct settings *settings, + struct job_list_entry *entry) { char *argv[4] = {}; size_t rootlen; @@ -871,6 +872,7 @@ static int execute_next_entry(struct execute_state *state, sigset_t mask; int outpipe[2] = { -1, -1 }; int errpipe[2] = { -1, -1 }; + int outfd, errfd; char name[32]; pid_t child; int result; @@ -884,9 +886,9 @@ static int execute_next_entry(struct execute_state *state, } if (!open_output_files(dirfd, outputs, true)) { - close(dirfd); fprintf(stderr, "Error opening output files\n"); - return -1; + result = -1; + goto out_dirfd; } if (settings->sync) { @@ -895,14 +897,9 @@ static int execute_next_entry(struct execute_state *state, } if (pipe(outpipe) || pipe(errpipe)) { - close_outputs(outputs); - close(dirfd); - close(outpipe[0]); - close(outpipe[1]); - close(errpipe[0]); - close(errpipe[1]); fprintf(stderr, "Error creating pipes: %s\n", strerror(errno)); - return -1; + result = -1; + goto out_pipe; } if ((kmsgfd = open("/dev/kmsg", O_RDONLY | O_CLOEXEC)) < 0) { @@ -923,14 +920,8 @@ static int execute_next_entry(struct execute_state *state, if (sigfd < 0) { /* TODO: Handle better */ fprintf(stderr, "Cannot monitor child process with signalfd\n"); - close(outpipe[0]); - close(errpipe[0]); - close(outpipe[1]); - close(errpipe[1]); - close(kmsgfd); - close_outputs(outputs); - close(dirfd); - return -1; + result = -1; + goto out_kmsgfd; } if (settings->log_level >= LOG_LEVEL_NORMAL) { @@ -951,21 +942,17 @@ static int execute_next_entry(struct execute_state *state, * Flush outputs before forking so our (buffered) output won't * end up in the test outputs. */ - fflush(stdout); fflush(stderr); - if ((child = fork())) { - int outfd = outpipe[0]; - int errfd = errpipe[0]; - close(outpipe[1]); - close(errpipe[1]); - - result = monitor_output(child, outfd, errfd, kmsgfd, sigfd, - outputs, time_spent, settings); - } else { - int outfd = outpipe[1]; - int errfd = errpipe[1]; + child = fork(); + if (child < 0) { + fprintf(stderr, "Failed to fork: %s\n", strerror(errno)); + result = -1; + goto out_kmsgfd; + } else if (child == 0) { + outfd = outpipe[1]; + errfd = errpipe[1]; close(outpipe[0]); close(errpipe[0]); @@ -974,13 +961,28 @@ static int execute_next_entry(struct execute_state *state, setenv("IGT_SENTINEL_ON_STDERR", "1", 1); execute_test_process(outfd, errfd, settings, entry); + /* unreachable */ } - /* TODO: Refactor this whole function to use onion teardown */ + outfd = outpipe[0]; + errfd = errpipe[0]; close(outpipe[1]); close(errpipe[1]); + outpipe[1] = errpipe[1] = -1; + + result = monitor_output(child, outfd, errfd, kmsgfd, sigfd, + outputs, time_spent, settings); + +out_kmsgfd: close(kmsgfd); +out_pipe: + close_outputs(outputs); + close(outpipe[0]); + close(outpipe[1]); + close(errpipe[0]); + close(errpipe[1]); close_outputs(outputs); +out_dirfd: close(dirfd); return result; -- cgit v1.2.3