[igt-dev] [PATCH i-g-t v2 4/5] runner: New test runner
Arkadiusz Hiler
arkadiusz.hiler at intel.com
Wed Jul 25 11:56:14 UTC 2018
On Wed, Jun 13, 2018 at 01:07:38PM +0300, Petri Latvala wrote:
<SNIP>
> diff --git a/meson.build b/meson.build
> index cd736d8e..9bf78eb1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -136,6 +136,7 @@ subdir('lib')
> subdir('tests')
> subdir('benchmarks')
> subdir('tools')
> +subdir('runner')
> if libdrm_intel.found()
> subdir('assembler')
> if ['x86', 'x86_64'].contains(host_machine.cpu_family())
> diff --git a/runner/executor.c b/runner/executor.c
> new file mode 100644
> index 00000000..8bc23339
> --- /dev/null
> +++ b/runner/executor.c
> @@ -0,0 +1,1022 @@
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/watchdog.h>
> +#include <signal.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +#include <sys/select.h>
> +#include <sys/signalfd.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <sys/utsname.h>
> +#include <sys/wait.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#include "igt_core.h"
> +#include "executor.h"
> +#include "output_strings.h"
> +
> +struct watchdogs
I think we can go with static and anonymous struct here to make it more
obvious that's intended global state.
> +{
> + int *fds;
> + size_t num_dogs;
> +} watchdogs;
And maybe an uppercase name?
<SNIP>
> +static bool prune_from_journal(struct job_list_entry *entry, int fd)
> +{
> + char *subtest;
> + FILE *f;
> + bool any_pruned = false;
> +
> + /*
> + * Each journal line is a subtest that has been started, or
> + * the line 'exit:$exitcode (time)', or 'timeout:$exitcode (time)'.
> + */
> +
> + f = fdopen(fd, "r");
> + if (!f)
> + return false;
> +
> + while (fscanf(f, "%ms", &subtest) == 1) {
> + if (!strncmp(subtest, "exit:", 5)) {
Defines/static consts for those magic strings? It's easier to strlen()
them.
> + /* Fully done. Mark that by making the binary name invalid. */
> + fscanf(f, " (%*fs)");
> + entry->binary[0] = '\0';
> + free(subtest);
> + continue;
> + }
> +
> + if (!strncmp(subtest, "timeout:", 8)) {
Same as above.
<SNIP>
> +static void dump_dmesg(int kmsgfd, int outfd)
> +{
> + /*
> + * Write kernel messages to the log file until we reach
> + * 'now'. Unfortunately, /dev/kmsg doesn't support seeking to
> + * -1 from SEEK_END so we need to use a second fd to read a
> + * message to match against, or stop when we reach EAGAIN.
> + */
> +
> + int comparefd = open("/dev/kmsg", O_RDONLY | O_NONBLOCK);
> + unsigned flags;
> + unsigned long long seq, cmpseq, usec;
> + char cont;
> + char buf[256];
> + ssize_t r;
> +
> + if (comparefd < 0)
> + return;
> +
> + if (fcntl(kmsgfd, F_SETFL, O_NONBLOCK))
According to man, success is indicated by "value other than -1".
> + return;
> +
> + while (1) {
> + if (comparefd >= 0) {
> + r = read(comparefd, buf, sizeof(buf) - 1);
> + if (r < 0) {
> + if (errno != EAGAIN && errno != EPIPE)
> + return;
> + } else {
> + buf[r] = '\0';
> + if (sscanf(buf, "%u,%llu,%llu,%c;",
> + &flags, &cmpseq, &usec, &cont) == 4) {
> + /* Reading comparison record done. */
> + close(comparefd);
> + comparefd = -1;
> + }
> + }
> + }
> +
> + r = read(kmsgfd, buf, sizeof(buf));
> + if (r <= 0) {
> + if (errno == EPIPE)
> + continue;
> +
> + /*
> + * If EAGAIN, we're done. If some other error,
> + * we can't do anything anyway.
> + */
> + return;
> + }
> +
> + write(outfd, buf, r);
> +
> + if (comparefd < 0 && sscanf(buf, "%u,%llu,%llu,%c;",
> + &flags, &seq, &usec, &cont) == 4) {
> + /*
> + * Comparison record has been read, compare
> + * the sequence number to see if we have read
> + * enough.
> + */
> + if (seq >= cmpseq)
> + return;
> + }
> + }
> +}
<SNIP>
> +/*
> + * Returns:
> + * =0 - Success
> + * <0 - Failure executing
> + * >0 - Timeout happened, need to recreate from journal
> + */
> +static int monitor_output(pid_t child,
> + int outfd, int errfd, int kmsgfd, int sigfd,
> + int *outputs,
> + struct settings *settings)
> +{
> + fd_set set;
> + char buf[256];
> + char *outbuf = NULL;
> + size_t outbufsize = 0;
> + char current_subtest[256] = {};
> + struct signalfd_siginfo siginfo;
> + ssize_t s;
> + int n, status;
> + int nfds = outfd;
> + int timeout = settings->inactivity_timeout;
> + int timeout_intervals = 1, intervals_left;
> + int wd_extra = 10;
> + int killed = 0; /* 0 if not killed, signal number otherwise */
> + struct timespec time_beg, time_end;
> + bool aborting = false;
> +
> + igt_gettime(&time_beg);
> +
> + if (errfd > nfds)
> + nfds = errfd;
> + if (kmsgfd > nfds)
> + nfds = kmsgfd;
> + if (sigfd > nfds)
> + nfds = sigfd;
> + nfds++;
> +
> + if (timeout > 0) {
> + /*
> + * Use original timeout plus some leeway. If we're still
> + * alive, we want to kill the test process instead of cutting
> + * power.
> + */
> + int wd_timeout = watchdogs_set_timeout(timeout + wd_extra);
> +
> + if (wd_timeout < timeout + wd_extra) {
> + /* Watchdog timeout smaller, so ping it more often */
> + if (wd_timeout - wd_extra < 0)
> + wd_extra = wd_timeout / 2;
> + timeout_intervals = timeout / (wd_timeout - wd_extra);
> + intervals_left = timeout_intervals;
> + timeout /= timeout_intervals;
> +
> + if (settings->log_level >= LOG_LEVEL_VERBOSE) {
> + printf("Watchdog doesn't support the timeout we requested (shortened to %d seconds).\n"
> + "Using %d intervals of %d seconds.\n",
> + wd_timeout, timeout_intervals, timeout);
> + }
> + }
> + }
> +
> + while (outfd >= 0 || errfd >= 0 || sigfd >= 0) {
> + struct timeval tv = { .tv_sec = timeout };
> +
> + FD_ZERO(&set);
> + if (outfd >= 0)
> + FD_SET(outfd, &set);
> + if (errfd >= 0)
> + FD_SET(errfd, &set);
> + if (kmsgfd >= 0)
> + FD_SET(kmsgfd, &set);
> + if (sigfd >= 0)
> + FD_SET(sigfd, &set);
> +
> + n = select(nfds, &set, NULL, NULL, timeout == 0 ? NULL : &tv);
> + if (n < 0) {
> + /* TODO */
> + return -1;
> + }
> +
> + if (n == 0) {
> + intervals_left--;
> + if (intervals_left) {
> + continue;
> + }
> +
> + ping_watchdogs();
> +
> + switch (killed) {
> + case 0:
> + if (settings->log_level >= LOG_LEVEL_NORMAL) {
> + printf("Timeout. Killing the current test with SIGTERM.\n");
> + }
> +
> + killed = SIGTERM;
> + if (!kill_child(killed, child))
> + return -1;
> +
> + /*
> + * Now continue the loop and let the
> + * dying child be handled normally.
> + */
> + timeout = 2; /* Timeout for waiting selected by fair dice roll. */
> + watchdogs_set_timeout(20);
> + intervals_left = timeout_intervals = 1;
> + break;
> + case SIGTERM:
> + if (settings->log_level >= LOG_LEVEL_NORMAL) {
> + printf("Timeout. Killing the current test with SIGKILL.\n");
> + }
> +
> + killed = SIGKILL;
> + if (!kill_child(killed, child))
> + return -1;
> +
> + intervals_left = timeout_intervals = 1;
> + break;
> + case SIGKILL:
> + /* Nothing that can be done, really. Let's tell the caller we want to abort. */
> + if (settings->log_level >= LOG_LEVEL_NORMAL) {
> + fprintf(stderr, "Child refuses to die. Aborting.\n");
> + }
> + close_watchdogs(settings);
> + free(outbuf);
> + close(outfd);
> + close(errfd);
> + close(kmsgfd);
> + close(sigfd);
> + return -1;
> + }
> +
> + continue;
> + }
> +
> + intervals_left = timeout_intervals;
> + ping_watchdogs();
> +
> + if (outfd >= 0 && FD_ISSET(outfd, &set)) {
I would just break this into smaller functions. You can extract the
whole stdout handling as a function of it's own. Same goes for the other
fds.
Then this monstrosity would read as a simple dispatcher with timeout
handling - much easier to digest.
If you have tried that and this turned out to be a bad idea, then I would
appreciate the explanation.
> + char *newline;
> +
> + s = read(outfd, buf, sizeof(buf));
> + if (s <= 0) {
> + if (s < 0) {
> + fprintf(stderr, "Error reading test's stdout: %s\n",
> + strerror(errno));
> + }
> +
> + close(outfd);
> + outfd = -1;
> + goto out_end;
> + }
> +
> + write(outputs[_F_OUT], buf, s);
> + if (settings->sync) {
> + fdatasync(outputs[_F_OUT]);
> + }
> +
> + outbuf = realloc(outbuf, outbufsize + s);
> + memcpy(outbuf + outbufsize, buf, s);
> + outbufsize += s;
> +
> + while ((newline = memchr(outbuf, '\n', outbufsize)) != NULL) {
> + size_t linelen = newline - outbuf + 1;
> +
> + if (linelen > strlen(starting_subtest) &&
> + !memcmp(outbuf, starting_subtest, strlen(starting_subtest))) {
Uppercase consts, please.
> + write(outputs[_F_JOURNAL], outbuf + strlen(starting_subtest),
> + linelen - strlen(starting_subtest));
> + memcpy(current_subtest, outbuf + strlen(starting_subtest),
> + linelen - strlen(starting_subtest));
> + current_subtest[linelen - strlen(starting_subtest)] = '\0';
> +
> + if (settings->log_level >= LOG_LEVEL_VERBOSE) {
> + fwrite(outbuf, 1, linelen, stdout);
> + }
> + }
> + if (linelen > strlen(subtest_result) &&
> + !memcmp(outbuf, subtest_result, strlen(subtest_result))) {
> + char *delim = memchr(outbuf, ':', linelen);
> +
> + if (delim != NULL) {
> + size_t subtestlen = delim - outbuf - strlen(subtest_result);
> + if (memcmp(current_subtest, outbuf + strlen(subtest_result),
> + subtestlen)) {
> + /* Result for a test that didn't ever start */
> + write(outputs[_F_JOURNAL],
> + outbuf + strlen(subtest_result),
> + subtestlen);
> + write(outputs[_F_JOURNAL], "\n", 1);
> + if (settings->sync) {
> + fdatasync(outputs[_F_JOURNAL]);
> + }
> + current_subtest[0] = '\0';
> + }
> +
> + if (settings->log_level >= LOG_LEVEL_VERBOSE) {
> + fwrite(outbuf, 1, linelen, stdout);
> + }
> + }
> + }
> +
> + memmove(outbuf, newline + 1, outbufsize - linelen);
> + outbufsize -= linelen;
> + }
> + }
> + out_end:
> +
> + if (errfd >= 0 && FD_ISSET(errfd, &set)) {
> + s = read(errfd, buf, sizeof(buf));
> + if (s <= 0) {
> + if (s < 0) {
> + fprintf(stderr, "Error reading test's stderr: %s\n",
> + strerror(errno));
> + }
> + close(errfd);
> + errfd = -1;
> + } else {
> + write(outputs[_F_ERR], buf, s);
> + if (settings->sync) {
> + fdatasync(outputs[_F_ERR]);
> + }
> + }
> + }
> +
> + if (kmsgfd >= 0 && FD_ISSET(kmsgfd, &set)) {
> + s = read(kmsgfd, buf, sizeof(buf));
> + if (s < 0) {
> + if (errno != EPIPE) {
> + fprintf(stderr, "Error reading from kmsg, stopping monitoring: %s\n",
> + strerror(errno));
> + close(kmsgfd);
> + kmsgfd = -1;
> + }
> + } else {
> + write(outputs[_F_DMESG], buf, s);
> + if (settings->sync) {
> + fdatasync(outputs[_F_DMESG]);
> + }
> + }
> + }
> +
> + if (sigfd >= 0 && FD_ISSET(sigfd, &set)) {
> + double time;
> +
> + s = read(sigfd, &siginfo, sizeof(siginfo));
> + if (s < 0) {
> + fprintf(stderr, "Error reading from signalfd: %s\n",
> + strerror(errno));
> + continue;
> + } else if (siginfo.ssi_signo == SIGCHLD) {
> + if (child != waitpid(child, &status, WNOHANG)) {
> + fprintf(stderr, "Failed to reap child\n");
> + status = 9999;
> + } else if (WIFEXITED(status)) {
> + status = WEXITSTATUS(status);
> + if (status >= 128) {
> + status = 128 - status;
> + }
> + } else if (WIFSIGNALED(status)) {
> + status = -WTERMSIG(status);
> + } else {
> + status = 9999;
> + }
> + } else {
> + /* We're dying, so we're taking them with us */
> + if (settings->log_level >= LOG_LEVEL_NORMAL)
> + printf("Abort requested, terminating children\n");
> +
> + aborting = true;
> + timeout = 2;
> + killed = SIGTERM;
> + if (!kill_child(killed, child))
> + return -1;
> +
> + continue;
> + }
> +
> + igt_gettime(&time_end);
> +
> + time = igt_time_elapsed(&time_beg, &time_end);
> + if (time < 0.0)
> + time = 0.0;
> +
> + if (!aborting) {
> + dprintf(outputs[_F_JOURNAL], "%s:%d (%.3fs)\n",
> + killed ? "timeout" : "exit",
> + status, time);
> + if (settings->sync) {
> + fdatasync(outputs[_F_JOURNAL]);
> + }
> + }
> +
> + close(sigfd);
> + sigfd = -1;
> + child = 0;
> + }
> + }
> +
> + dump_dmesg(kmsgfd, outputs[_F_DMESG]);
> + if (settings->sync)
> + fdatasync(outputs[_F_DMESG]);
> +
> + free(outbuf);
> + close(outfd);
> + close(errfd);
> + close(kmsgfd);
> + close(sigfd);
> +
> + if (aborting)
> + return -1;
> +
> + return killed;
> +}
> +
<SNIP>
> +/*
> + * Returns:
> + * =0 - Success
> + * <0 - Failure executing
> + * >0 - Timeout happened, need to recreate from journal
> + */
> +static int execute_entry(size_t idx,
> + size_t total,
> + struct settings *settings,
> + struct job_list_entry *entry,
> + int testdirfd, int resdirfd)
> +{
> + int dirfd;
> + int outputs[_F_LAST];
> + int kmsgfd;
> + int sigfd;
> + sigset_t mask;
> + int outpipe[2] = { -1, -1 };
> + int errpipe[2] = { -1, -1 };
> + char name[32];
> + pid_t child;
> + int result;
> +
> + snprintf(name, 32, "%zd", idx);
sizeof
> + mkdirat(resdirfd, name, 0777);
> + if ((dirfd = openat(resdirfd, name, O_DIRECTORY | O_RDONLY | O_CLOEXEC)) < 0) {
> + fprintf(stderr, "Error accessing individual test result directory\n");
> + return -1;
> + }
> +
> + if (!open_output_files(dirfd, outputs, true)) {
> + close(dirfd);
> + fprintf(stderr, "Error opening output files\n");
> + return -1;
> + }
> +
> + if (settings->sync) {
> + fsync(dirfd);
> + fsync(resdirfd);
> + }
> +
> + 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;
> + }
> +
> + if ((kmsgfd = open("/dev/kmsg", O_RDONLY | O_CLOEXEC)) < 0) {
> + fprintf(stderr, "Warning: Cannot open /dev/kmsg\n");
> + } else {
> + /* TODO: Checking of abort conditions in pre-execute dmesg */
> + lseek(kmsgfd, 0, SEEK_END);
> + }
> +
> + sigemptyset(&mask);
> + sigaddset(&mask, SIGCHLD);
> + sigaddset(&mask, SIGINT);
> + sigaddset(&mask, SIGTERM);
> + sigaddset(&mask, SIGQUIT);
> + sigprocmask(SIG_BLOCK, &mask, NULL);
> + sigfd = signalfd(-1, &mask, O_CLOEXEC);
> +
> + 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;
> + }
> +
> + if (settings->log_level >= LOG_LEVEL_NORMAL) {
> + int width = digits(total);
> + printf("[%0*zd/%0*zd] %s", width, idx + 1, width, total, entry->binary);
> + if (entry->subtest_count > 0) {
> + size_t i;
> + char *delim = "";
> +
> + printf(" (");
> + for (i = 0; i < entry->subtest_count; i++) {
> + printf("%s%s", delim, entry->subtests[i]);
> + delim = ", ";
> + }
> + printf(")");
> + }
> + printf("\n");
> + }
> +
> + 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, settings);
> + } else {
> + int outfd = outpipe[1];
> + int errfd = errpipe[1];
> + close(outpipe[0]);
> + close(errpipe[0]);
> +
> + sigprocmask(SIG_UNBLOCK, &mask, NULL);
> +
> + setenv("IGT_SENTINEL_ON_STDERR", "1", 1);
> +
> + execute_test_process(outfd, errfd, settings, entry);
> + }
> +
> + close(outpipe[1]);
> + close(errpipe[1]);
> + close(kmsgfd);
> + close_outputs(outputs);
> + close(dirfd);
onion teardown? with gotos and all the jazz
> +
> + return result;
> +}
> +
> +static int remove_file(int dirfd, char *name)
> +{
> + return unlinkat(dirfd, name, 0) && errno != ENOENT;
> +}
> +
> +static bool clear_test_result_directory(int dirfd)
> +{
> + int i;
> +
> + for (i = 0; i < _F_LAST; i++) {
> + if (remove_file(dirfd, filenames[i])) {
> + fprintf(stderr, "Error deleting %s from test result directory: %s\n",
> + filenames[i],
> + strerror(errno));
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> +static bool clear_old_results(char *path)
> +{
> + int dirfd;
> + size_t i;
> +
> + if ((dirfd = open(path, O_DIRECTORY | O_RDONLY)) < 0) {
> + if (errno == ENOENT) {
> + /* Successfully cleared if it doesn't even exist */
> + return true;
> + }
> +
> + fprintf(stderr, "Error clearing old results: %s\n", strerror(errno));
> + return false;
> + }
> +
> + if (unlinkat(dirfd, "uname.txt", 0) && errno != ENOENT) {
> + close(dirfd);
> + fprintf(stderr, "Error clearing old results: %s\n", strerror(errno));
> + return false;
> + }
> +
> + for (i = 0; true; i++) {
> + char name[32];
> + int resdirfd;
> +
> + snprintf(name, sizeof(name), "%zd", i);
> + if ((resdirfd = openat(dirfd, name, O_DIRECTORY | O_RDONLY)) < 0)
> + break;
> +
> + if (!clear_test_result_directory(resdirfd)) {
> + close(resdirfd);
> + close(dirfd);
> + return false;
> + }
> + close(resdirfd);
> + if (unlinkat(dirfd, name, AT_REMOVEDIR)) {
> + fprintf(stderr,
> + "Warning: Result directory %s contains extra files\n",
> + name);
> + }
> + }
> +
> + close(dirfd);
> +
> + return true;
> +}
> +
> +bool initialize_execute_state_from_resume(int dirfd,
> + struct execute_state *state,
> + struct settings *settings,
> + struct job_list *list)
> +{
> + struct job_list_entry *entry;
> + int resdirfd, fd, i;
> +
> + free_settings(settings);
> + free_job_list(list);
> + memset(state, 0, sizeof(*state));
> +
> + if (!read_settings(settings, dirfd) ||
> + !read_job_list(list, dirfd)) {
> + close(dirfd);
> + return false;
> + }
> +
> + for (i = list->size; i >= 0; i--) {
> + char name[32];
> +
> + snprintf(name, 32, "%d", i);
sizeof
> + if ((resdirfd = openat(dirfd, name, O_DIRECTORY | O_RDONLY)) >= 0) {
> + break;
> + }
> + }
> +
> + if (i < 0) {
> + /* Nothing has been executed yet, state is fine as is */
> + goto success;
> + }
kinda uneccesray {}
same for the if a little bit up
> +
> + entry = &list->entries[i];
> + state->next = i;
> + if ((fd = openat(resdirfd, filenames[_F_JOURNAL], O_RDONLY)) >= 0) {
> + if (!prune_from_journal(entry, fd)) {
> + /*
> + * The test does not have subtests, or
> + * incompleted before the first subtest
> + * began. Either way, not suitable to
> + * re-run.
> + */
> + state->next = i + 1;
> + } else if (entry->binary[0] == '\0') {
> + /* This test is fully completed */
> + state->next = i + 1;
> + }
> +
> + close(fd);
> + }
> +
> + success:
> + close(resdirfd);
> + close(dirfd);
> +
> + return true;
> +}
> +
> +bool initialize_execute_state(struct execute_state *state,
> + struct settings *settings,
> + struct job_list *job_list)
> +{
> + memset(state, 0, sizeof(*state));
> +
> + if (!validate_settings(settings))
> + return false;
> +
> + if (!serialize_settings(settings) ||
> + !serialize_job_list(job_list, settings))
> + return false;
> +
> + if (settings->overwrite &&
> + !clear_old_results(settings->results_path))
> + return false;
> +
> + return true;
> +}
> +
> +bool execute(struct execute_state *state,
> + struct settings *settings,
> + struct job_list *job_list)
> +{
> + struct utsname unamebuf;
> + int resdirfd, testdirfd, unamefd;
> +
> + if ((resdirfd = open(settings->results_path, O_DIRECTORY | O_RDONLY)) < 0) {
> + /* Initialize state should have done this */
> + fprintf(stderr, "Error: Failure opening results path %s\n",
> + settings->results_path);
> + return false;
> + }
> +
> + if ((testdirfd = open(settings->test_root, O_DIRECTORY | O_RDONLY)) < 0) {
> + fprintf(stderr, "Error: Failure opening test root %s\n",
> + settings->test_root);
> + close(resdirfd);
> + return false;
> + }
> +
> + /* TODO: On resume, don't rewrite, verify that content matches current instead */
> + if ((unamefd = openat(resdirfd, "uname.txt", O_CREAT | O_WRONLY | O_TRUNC, 0666)) < 0) {
> + fprintf(stderr, "Error: Failure creating opening uname.txt: %s\n",
> + strerror(errno));
> + close(testdirfd);
> + close(resdirfd);
> + return false;
> + }
> +
> + init_watchdogs(settings);
> +
> + if (!uname(&unamebuf)) {
> + dprintf(unamefd, "%s %s %s %s %s\n",
> + unamebuf.sysname,
> + unamebuf.nodename,
> + unamebuf.release,
> + unamebuf.version,
> + unamebuf.machine);
> + } else {
> + dprintf(unamefd, "uname() failed\n");
> + }
> + close(unamefd);
> +
> + for (; state->next < job_list->size;
> + state->next++) {
> + int result = execute_entry(state->next,
> + job_list->size,
> + settings,
> + &job_list->entries[state->next],
> + testdirfd, resdirfd);
> + if (result != 0) {
> + close(testdirfd);
> + close_watchdogs(settings);
> + if (result > 0) {
> + initialize_execute_state_from_resume(resdirfd, state, settings, job_list);
> + return execute(state, settings, job_list);
> + }
missing close(resdirfd) ?
> + return false;
> + }
> + }
> +
> + close(testdirfd);
> + close(resdirfd);
> + close_watchdogs(settings);
> + return true;
> +}
<SNIP>
> diff --git a/runner/job_list.c b/runner/job_list.c
> new file mode 100644
> index 00000000..3f2c8e68
> --- /dev/null
> +++ b/runner/job_list.c
> @@ -0,0 +1,487 @@
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <linux/limits.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include "job_list.h"
> +#include "igt_core.h"
> +
> +static bool matches_any(const char *str, struct regex_list *list)
> +{
> + size_t i;
> +
> + for (i = 0; i < list->size; i++) {
> + if (regexec(list->regexes[i], str,
> + (size_t)0, NULL, 0) == 0) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static void add_job_list_entry(struct job_list *job_list,
> + char *binary,
> + char **subtests,
> + size_t subtest_count)
> +{
> + struct job_list_entry *entry;
> +
> + job_list->size++;
> + job_list->entries = realloc(job_list->entries, job_list->size * sizeof(*job_list->entries));
> + entry = &job_list->entries[job_list->size - 1];
> +
> + entry->binary = binary;
> + entry->subtests = subtests;
> + entry->subtest_count = subtest_count;
> +}
> +
> +static void add_subtests(struct job_list *job_list, struct settings *settings,
> + char *binary,
> + struct regex_list *include, struct regex_list *exclude)
> +{
> + FILE *p;
> + char cmd[256] = {};
> + char *subtestname;
> + char **subtests = NULL;
> + size_t num_subtests = 0;
> + int s;
> +
> + s = snprintf(cmd, sizeof(cmd), "%s/%s --list-subtests",
> + settings->test_root, binary);
> + if (s < 0) {
> + fprintf(stderr, "Failure generating command string, this shouldn't happen.\n");
> + return;
> + }
> +
> + if (s >= sizeof(cmd)) {
> + fprintf(stderr, "Path to binary too long, ignoring: %s/%s\n",
> + settings->test_root, binary);
> + return;
> + }
> +
> + p = popen(cmd, "r");
> + if (!p) {
> + fprintf(stderr, "popen failed when executing %s: %s\n",
> + cmd,
> + strerror(errno));
> + return;
> + }
> +
> + while (fscanf(p, "%ms", &subtestname) == 1) {
> + if (exclude && exclude->size && matches_any(subtestname, exclude)) {
> + free(subtestname);
> + continue;
> + }
> +
> + if (include && include->size && !matches_any(subtestname, include)) {
> + free(subtestname);
> + continue;
> + }
> +
> + if (settings->multiple_mode) {
> + num_subtests++;
> + subtests = realloc(subtests, num_subtests * sizeof(*subtests));
> + subtests[num_subtests - 1] = strdup(subtestname);
> + } else {
> + subtests = malloc(sizeof(*subtests));
> + *subtests = strdup(subtestname);
> + add_job_list_entry(job_list, strdup(binary), subtests, 1);
> + subtests = NULL;
> + }
> +
> + free(subtestname);
> + }
> +
> + if (num_subtests) {
> + add_job_list_entry(job_list, strdup(binary), subtests, num_subtests);
> + }
Unecessary {}
> +
> + s = pclose(p);
> + if (s == 0) {
> + return;
> + } else if (s == -1) {
> + fprintf(stderr, "popen error when executing %s: %s\n", binary, strerror(errno));
> + } else if (WIFEXITED(s)) {
> + if (WEXITSTATUS(s) == IGT_EXIT_INVALID) {
> + /* No subtests on this one */
> + if (exclude && exclude->size && matches_any(binary, exclude)) {
> + return;
> + }
> + if (!include || !include->size || matches_any(binary, include)) {
> + add_job_list_entry(job_list, strdup(binary), NULL, 0);
> + return;
> + }
> + }
> + } else {
> + fprintf(stderr, "Test binary %s died unexpectedly\n", binary);
> + }
> +}
> +
> +static bool filtered_job_list(struct job_list *job_list,
> + struct settings *settings,
> + int fd)
> +{
> + FILE *f;
> + char buf[128];
> +
> + if (job_list->entries != NULL) {
> + fprintf(stderr, "Caller didn't clear the job list, this shouldn't happen\n");
> + exit(1);
> + }
> +
> + f = fdopen(fd, "r");
> +
> + while (fscanf(f, "%127s", buf) == 1) {
> + if (!strcmp(buf, "TESTLIST") || !(strcmp(buf, "END")))
> + continue;
> +
> + /*
> + * If the binary name matches exclude filters, no
> + * subtests are added.
> + */
> + if (settings->exclude_regexes.size && matches_any(buf, &settings->exclude_regexes))
> + continue;
> +
> + /*
> + * If the binary name matches include filters (or include filters not present),
> + * all subtests except those matching exclude filters are added.
> + */
> + if (!settings->include_regexes.size || matches_any(buf, &settings->include_regexes)) {
> + if (settings->multiple_mode && !settings->exclude_regexes.size)
> + /*
> + * Optimization; we know that all
> + * subtests will be included, so we
> + * get to omit executing
> + * --list-subtests.
> + */
> + add_job_list_entry(job_list, strdup(buf), NULL, 0);
> + else
> + add_subtests(job_list, settings, buf,
> + NULL, &settings->exclude_regexes);
> + continue;
> + }
> +
> + /*
> + * Binary name doesn't match exclude or include filters.
> + */
> + add_subtests(job_list, settings, buf,
> + &settings->include_regexes,
> + &settings->exclude_regexes);
> + }
> +
> + return job_list->size != 0;
> +}
> +
> +static bool job_list_from_test_list(struct job_list *job_list,
> + struct settings *settings)
> +{
> + FILE *f;
> + char *line = NULL;
> + size_t line_len = 0;
> + struct job_list_entry entry = {};
> + bool any = false;
> +
> + if ((f = fopen(settings->test_list, "r")) == NULL) {
> + fprintf(stderr, "Cannot open test list file %s\n", settings->test_list);
> + return false;
> + }
> +
> + while (1) {
> + char *binary;
> + char *delim;
> +
> + if (getline(&line, &line_len, f) == -1) {
> + if (errno == EINTR)
> + continue;
> + else
> + break;
> + }
> +
> + /* # starts a comment */
> + if ((delim = strchr(line, '#')) != NULL) {
> + *delim = '\0';
> + }
Unecessary {}
And many more, just ag for 'if .* {\n.*\n.*}'
> +
> + if (sscanf(line, "igt@%ms", &binary) == 1) {
> + if ((delim = strchr(binary, '@')) != NULL) {
> + *delim++ = '\0';
> + }
> +
> + if (!settings->multiple_mode) {
> + char **subtests = NULL;
> + if (delim) {
> + subtests = malloc(sizeof(char*));
> + subtests[0] = strdup(delim);
> + }
> + add_job_list_entry(job_list, strdup(binary),
> + subtests, (size_t)(subtests != NULL));
> + any = true;
> + free(binary);
> + binary = NULL;
> + continue;
> + }
> +
> + /*
> + * If the currently built entry has the same
> + * binary, add a subtest. Otherwise submit
> + * what's already built and start a new one.
> + */
> + if (entry.binary && !strcmp(entry.binary, binary)) {
> + if (!delim) {
> + /* ... except we didn't get a subtest */
> + fprintf(stderr,
> + "Error: Unexpected test without subtests "
> + "after same test had subtests\n");
> + free(binary);
> + fclose(f);
> + return false;
> + }
> + entry.subtest_count++;
> + entry.subtests = realloc(entry.subtests,
> + entry.subtest_count *
> + sizeof(*entry.subtests));
> + entry.subtests[entry.subtest_count - 1] = strdup(delim);
> + free(binary);
> + binary = NULL;
> + continue;
> + }
> +
> + if (entry.binary) {
> + add_job_list_entry(job_list, entry.binary, entry.subtests, entry.subtest_count);
> + any = true;
> + }
> +
> + memset(&entry, 0, sizeof(entry));
> + entry.binary = strdup(binary);
> + if (delim) {
> + entry.subtests = malloc(sizeof(*entry.subtests));
> + entry.subtests[0] = strdup(delim);
> + entry.subtest_count = 1;
> + }
> +
> + free(binary);
> + binary = NULL;
> + }
> + }
> +
> + if (entry.binary) {
> + add_job_list_entry(job_list, entry.binary, entry.subtests, entry.subtest_count);
> + any = true;
> + }
> +
> + free(line);
> + fclose(f);
> + return any;
> +}
<SNIP>
> diff --git a/runner/output_strings.h b/runner/output_strings.h
> new file mode 100644
> index 00000000..81248d49
> --- /dev/null
> +++ b/runner/output_strings.h
> @@ -0,0 +1,38 @@
> +#ifndef RUNNER_OUTPUT_STRINGS_H
> +#define RUNNER_OUTPUT_STRINGS_H
> +
> +/*
> + * Output when a subtest has begun. Is followed by the subtest name.
> + *
> + * Example:
> + * Starting subtest: subtestname
> + */
> +static const char starting_subtest[] = "Starting subtest: ";
> +
> +/*
> + * Output when a subtest has ended. Is followed by the subtest name
> + * and optionally its runtime.
> + *
> + * Examples:
> + * Subtest subtestname: SKIP
> + * Subtest subtestname: PASS (0.003s)
> + */
> +static const char subtest_result[] = "Subtest ";
> +
> +/*
> + * Output in dmesg when a subtest has begin. Is followed by the subtest name.
> + *
> + * Example:
> + * [IGT] test-binary-name: starting subtest subtestname
> + */
> +static const char starting_subtest_dmesg[] = ": starting subtest ";
> +
> +/*
> + * Output when a test process is executed.
> + *
> + * Example:
> + * IGT-Version: 1.22-gde9af343 (x86_64) (Linux: 4.12.0-1-amd64 x86_64)
> + */
> +static const char versionstring[] = "IGT-Version: ";
What about uppercase consts? :P
> +
> +#endif
> diff --git a/runner/resultgen.c b/runner/resultgen.c
> new file mode 100644
> index 00000000..e0466446
> --- /dev/null
> +++ b/runner/resultgen.c
> @@ -0,0 +1,954 @@
> +#include <ctype.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
> +#include <unistd.h>
> +
> +#include <json.h>
> +
> +#include "igt_core.h"
> +#include "resultgen.h"
> +#include "settings.h"
> +#include "executor.h"
> +#include "output_strings.h"
> +
> +struct subtests
> +{
> + char **names;
> + size_t size;
> +};
> +
> +/*
> + * A lot of string handling here operates on an mmapped buffer, and
> + * thus we can't assume nul-terminated strings. Buffers will be passed
'null'
> + * around as pointer+size, or pointer+pointer-past-the-end, the mem*()
> + * family of functions is used instead of str*().
> + */
> +
> +static char *find_line_starting_with(char *haystack, const char *needle, char *end)
> +{
> + while (haystack < end) {
> + char *line_end = memchr(haystack, '\n', end - haystack);
> +
> + if (end - haystack < strlen(needle))
> + return NULL;
> + if (!memcmp(haystack, needle, strlen(needle)))
> + return haystack;
> + if (line_end == NULL)
> + return NULL;
> + haystack = line_end + 1;
> + }
> +
> + return NULL;
> +}
> +
> +static char *find_line_starting_with_either(char *haystack,
> + const char *needle1,
> + const char *needle2,
> + char *end)
> +{
> + while (haystack < end) {
> + char *line_end = memchr(haystack, '\n', end - haystack);
> + size_t linelen = line_end != NULL ? line_end - haystack : end - haystack;
> + if ((linelen >= strlen(needle1) && !memcmp(haystack, needle1, strlen(needle1))) ||
> + (linelen >= strlen(needle2) && !memcmp(haystack, needle2, strlen(needle2))))
> + return haystack;
> +
> + if (line_end == NULL)
> + return NULL;
> +
> + haystack = line_end + 1;
> + }
> +
> + return NULL;
> +}
> +
> +static char *next_line(char *line, char *bufend)
> +{
> + char *ret;
> +
> + if (!line)
> + return NULL;
> +
> + ret = memchr(line, '\n', bufend - line);
> + if (ret)
> + ret++;
> +
> + if (ret < bufend)
> + return ret;
> + else
> + return NULL;
> +}
> +
> +static char *find_line_after_last(char *begin,
> + const char *needle1,
> + const char *needle2,
> + char *end)
> +{
> + char *one, *two;
> + char *needle1_newline = malloc(strlen(needle1) + 2);
> + char *needle2_newline = malloc(strlen(needle2) + 2);
> +
> + needle1_newline[0] = needle2_newline[0] = '\n';
> + strcpy(needle1_newline + 1, needle1);
> + strcpy(needle2_newline + 1, needle2);
> +
> + while (true) {
> + one = memmem(begin, end - begin, needle1_newline, strlen(needle1_newline));
> + two = memmem(begin, end - begin, needle2_newline, strlen(needle2_newline));
> + if (one == NULL && two == NULL)
> + break;
> +
> + if (one != NULL && begin < one)
> + begin = one;
> + if (two != NULL && begin < two)
> + begin = two;
> +
> + one = next_line(begin, end);
> + if (one != NULL)
> + begin = one;
> + }
> + free(needle1_newline);
> + free(needle2_newline);
> +
> + one = memchr(begin, '\n', end - begin);
> + if (one != NULL)
> + return ++one;
> +
> + return begin;
name "begin" seems odd in the context other than the function prototype.
char *current_pos = begin; /* or something? */
> +}
> +
> +static size_t count_lines(const char *buf, const char *bufend)
> +{
> + size_t ret = 0;
> + while (buf < bufend && (buf = memchr(buf, '\n', bufend - buf)) != NULL) {
> + ret++;
> + buf++;
> + }
> +
> + return ret;
> +}
> +
> +static char *lowercase(char *str)
> +{
> + char *ret = malloc(strlen(str) + 1);
> + char *q = ret;
> +
> + while (*str) {
> + if (isspace(*str))
> + break;
> +
> + *q++ = tolower(*str++);
> + }
> + *q = '\0';
> +
> + return ret;
> +}
> +
> +static void append_line(char **buf, size_t *buflen, char *line)
> +{
> + size_t linelen = strlen(line);
> +
> + *buf = realloc(*buf, *buflen + linelen + 1);
> + strcpy(*buf + *buflen, line);
> + *buflen += linelen;
> +}
> +
> +static char *generate_piglit_name(char *binary, char *subtest)
> +{
> + static char namebuf[256];
> +
> + char *lc_binary = lowercase(binary);
> + char *lc_subtest = NULL;
> +
> + if (!subtest) {
> + snprintf(namebuf, sizeof(namebuf), "igt@%s", lc_binary);
> + free(lc_binary);
> + return namebuf;
> + }
> +
> + lc_subtest = lowercase(subtest);
> +
> + snprintf(namebuf, sizeof(namebuf), "igt@%s@%s", lc_binary, lc_subtest);
> +
> + free(lc_binary);
> + free(lc_subtest);
> + return namebuf;
> +}
pass namebuf as a function argument, instead of returning pointer to the
static memory here, so the ownership of memory is obvious from the caller?
> +
> +static struct {
const
> + char *output_str;
> + char *result_str;
> +} resultmap[] = {
> + { "SUCCESS", "pass" },
> + { "SKIP", "skip" },
> + { "FAIL", "fail" },
> + { "CRASH", "crash" },
> + { "TIMEOUT", "timeout" },
> +};
> +static void parse_result_string(char *resultstring, size_t len, char **result, double *time)
> +{
> + size_t i;
> + size_t wordlen = 0;
> +
> + while (wordlen < len && !isspace(resultstring[wordlen])) {
> + wordlen++;
> + }
> +
> + *result = NULL;
> + for (i = 0; i < (sizeof(resultmap) / sizeof(resultmap[0])); i++) {
> + if (!strncmp(resultstring, resultmap[i].output_str, wordlen)) {
> + *result = resultmap[i].result_str;
> + break;
> + }
> + }
> +
> + /* If the result string is unknown, use incomplete */
> + if (!*result)
> + *result = "incomplete";
> +
> + /*
> + * Check for subtest runtime after the result. The string is
> + * '(' followed by the runtime in seconds as floating point,
> + * followed by 's)'.
> + */
> + wordlen++;
> + if (wordlen < len && resultstring[wordlen] == '(') {
> + wordlen++;
> + char *dup = malloc(len - wordlen + 1);
> + memcpy(dup, resultstring + wordlen, len - wordlen);
> + dup[len - wordlen] = '\0';
> + *time = strtod(dup, NULL);
> +
> + free(dup);
> + }
> +}
> +
> +static void parse_subtest_result(char *subtest, char **result, double *time, char *buf, char *bufend)
> +{
> + char *line;
> + char *line_end;
> + char *resultstring;
> + size_t linelen;
> + size_t subtestlen = strlen(subtest);
> +
> + *result = NULL;
> + *time = 0.0;
> +
> + if (!buf) return;
> +
> + /*
> + * The result line structure is:
> + *
> + * - The string "Subtest " (`subtest_result` from output_strings.h)
> + * - The subtest name
> + * - The characters ':' and ' '
> + * - Subtest result string
> + * - Optional:
> + * -- The characters ' ' and '('
> + * -- Subtest runtime in seconds as floating point
> + * -- The characters 's' and ')'
> + *
> + * Example:
> + * Subtest subtestname: PASS (0.003s)
> + */
> +
> + line = find_line_starting_with(buf, subtest_result, bufend);
> + if (!line) {
> + *result = "incomplete";
> + return;
> + }
> +
> + line_end = memchr(line, '\n', bufend - line);
> + linelen = line_end != NULL ? line_end - line : bufend - line;
> +
> + if (strlen(subtest_result) + subtestlen + strlen(": ") > linelen ||
> + strncmp(line + strlen(subtest_result), subtest, subtestlen))
> + return parse_subtest_result(subtest, result, time, line + linelen, bufend);
> +
> + resultstring = line + strlen(subtest_result) + subtestlen + strlen(": ");
> + parse_result_string(resultstring, linelen - (resultstring - line), result, time);
> +}
> +
> +static struct json_object *get_or_create_json_object(struct json_object *base,
> + char *key)
> +{
> + struct json_object *ret;
> +
> + if (json_object_object_get_ex(base, key, &ret))
> + return ret;
> +
> + ret = json_object_new_object();
> + json_object_object_add(base, key, ret);
> +
> + return ret;
> +}
> +
> +static void set_result(struct json_object *obj, char *result)
> +{
> + json_object_object_add(obj, "result",
> + json_object_new_string(result));
> +}
> +
> +static void add_runtime(struct json_object *obj, double time)
> +{
> + double oldtime;
> + struct json_object *timeobj = get_or_create_json_object(obj, "time");
> + struct json_object *oldend;
> +
> + json_object_object_add(timeobj, "__type__",
> + json_object_new_string("TimeAttribute"));
> + json_object_object_add(timeobj, "start",
> + json_object_new_double(0.0));
> +
> + if (!json_object_object_get_ex(timeobj, "end", &oldend)) {
> + json_object_object_add(timeobj, "end",
> + json_object_new_double(time));
> + return;
> + }
> +
> + /* Add the runtime to the existing runtime. */
> + oldtime = json_object_get_double(oldend);
> + time += oldtime;
> + json_object_object_add(timeobj, "end",
> + json_object_new_double(time));
> +}
> +
> +static void set_runtime(struct json_object *obj, double time)
> +{
> + struct json_object *timeobj = get_or_create_json_object(obj, "time");
> +
> + json_object_object_add(timeobj, "__type__",
> + json_object_new_string("TimeAttribute"));
> + json_object_object_add(timeobj, "start",
> + json_object_new_double(0.0));
> + json_object_object_add(timeobj, "end",
> + json_object_new_double(time));
> +}
> +
> +static bool fill_from_output(int fd, char *binary, char *key,
> + struct subtests *subtests,
> + struct json_object *tests)
> +{
> + char *buf, *bufend;
> + struct stat statbuf;
> + char *piglit_name = NULL;
> + char *igt_version = NULL;
> + size_t igt_version_len = 0;
> + struct json_object *current_test = NULL;
> + size_t i;
> +
> + if (fstat(fd, &statbuf))
> + return false;
> +
> + if (statbuf.st_size != 0) {
> + buf = mmap(NULL, statbuf.st_size, PROT_READ, MAP_SHARED, fd, 0);
> + if (buf == MAP_FAILED)
> + return false;
> + } else {
> + buf = NULL;
> + }
> +
> + bufend = buf + statbuf.st_size;
> +
> + igt_version = find_line_starting_with(buf, versionstring, bufend);
> + if (igt_version) {
> + char *newline = memchr(igt_version, '\n', bufend - igt_version);
> + igt_version_len = newline - igt_version;
> + }
> +
> + if (subtests->size == 0) {
> + /* No subtests */
> + piglit_name = generate_piglit_name(binary, NULL);
> + current_test = get_or_create_json_object(tests, piglit_name);
> +
> + json_object_object_add(current_test, key,
> + json_object_new_string_len(buf, statbuf.st_size));
> + if (igt_version)
> + json_object_object_add(current_test, "igt-version",
> + json_object_new_string_len(igt_version,
> + igt_version_len));
> +
> + return true;
This function runs long and is hard to digest in one reading.
You can make a helper function out of this, to handle no subtests case.
> + }
> +
> + for (i = 0; i < subtests->size; i++) {
> + char *this_sub_begin, *this_sub_result;
> + char *resulttext;
> + char *beg, *end, *startline;
> + double time;
> + int begin_len;
> + int result_len;
> +
> + piglit_name = generate_piglit_name(binary, subtests->names[i]);
> + current_test = get_or_create_json_object(tests, piglit_name);
> +
> + begin_len = asprintf(&this_sub_begin, "%s%s\n", starting_subtest, subtests->names[i]);
> + result_len = asprintf(&this_sub_result, "%s%s: ", subtest_result, subtests->names[i]);
> +
> + if (begin_len < 0 || result_len < 0) {
> + fprintf(stderr, "Failure generating strings\n");
> + return false;
> + }
> +
> + beg = find_line_starting_with(buf, this_sub_begin, bufend);
> + end = find_line_starting_with(buf, this_sub_result, bufend);
> + startline = beg;
> +
> + free(this_sub_begin);
> + free(this_sub_result);
> +
> + if (beg == NULL && end == NULL) {
> + /* No output at all */
> + beg = bufend;
> + end = bufend;
> + }
> +
> + if (beg == NULL) {
> + /*
> + * Subtest didn't start, probably skipped from
> + * fixture already. Start from the result
> + * line, it gets adjusted below.
> + */
> + beg = end;
> + }
> +
> + /* Include the output after the previous subtest output */
> + beg = find_line_after_last(buf,
> + starting_subtest,
> + subtest_result,
> + beg);
> +
> + if (end == NULL) {
> + /* Incomplete result. Find the next starting subtest or result. */
> + end = next_line(startline, bufend);
> + if (end != NULL) {
> + end = find_line_starting_with_either(end,
> + starting_subtest,
> + subtest_result,
> + bufend);
> + }
> + if (end == NULL) {
> + end = bufend;
> + }
> + } else {
> + /*
> + * Now pointing to the line where this sub's
> + * result is. We need to include that of
> + * course.
> + */
> + char *nexttest = next_line(end, bufend);
> +
> + /* Stretch onwards until the next subtest begins or ends */
> + if (nexttest != NULL) {
> + nexttest = find_line_starting_with_either(nexttest,
> + starting_subtest,
> + subtest_result,
> + bufend);
> + }
> + if (nexttest != NULL) {
> + end = nexttest;
> + } else {
> + end = bufend;
> + }
> + }
> +
> + json_object_object_add(current_test, key,
> + json_object_new_string_len(beg, end - beg));
> +
> + if (igt_version) {
> + json_object_object_add(current_test, "igt-version",
> + json_object_new_string_len(igt_version,
> + igt_version_len));
> + }
> +
> + if (!json_object_object_get_ex(current_test, "result", NULL)) {
> + parse_subtest_result(subtests->names[i], &resulttext, &time, beg, end);
> + set_result(current_test, resulttext);
> + set_runtime(current_test, time);
> + }
And another one for handling a subtest.
Bonus points for splitting it even further :P
> + }
> +
> + return true;
> +}
> +
> +/*
> + * This regexp controls the kmsg handling. All kernel log records that
> + * have log level of warning or higher convert the result to
> + * dmesg-warn/dmesg-fail unless they match this regexp.
> + *
> + * TODO: Move this to external files, i915-suppressions.txt,
> + * general-suppressions.txt et al.
> + */
<SNIP>
Whew!
With the missing close():
Reviewed-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
All the other comments are cosmetic.
--
Cheers,
Arek
More information about the igt-dev
mailing list