From 0f0e60a8f2e21a37624d45af4b0b168477fe6f67 Mon Sep 17 00:00:00 2001 From: Ken Hornstein Date: Wed, 17 May 2017 21:00:47 -0400 Subject: [PATCH] Switch from using script(1) to a new custom utility, runpty. It turns out on some systems script(1) has a bug that results in hangs that is simply too difficult to test for, and this custom utility is simpler. --- .gitignore | 1 + Makefile.am | 4 + test/install-mh/test-version-check | 56 ++------- test/runpty.c | 175 +++++++++++++++++++++++++++++ 4 files changed, 187 insertions(+), 49 deletions(-) create mode 100644 test/runpty.c diff --git a/.gitignore b/.gitignore index 5e313f85..a3bdb9c0 100644 --- a/.gitignore +++ b/.gitignore @@ -122,6 +122,7 @@ a.out.dSYM/ /test/getcanon /test/getcwidth /test/getfullname +/test/runpty /test/*.exe # Removed by mostlyclean: diff --git a/Makefile.am b/Makefile.am index f32d98bb..4f6627e4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -177,6 +177,7 @@ check_PROGRAMS = \ test/getcanon \ test/getcwidth \ test/getfullname \ + test/runpty \ # ## So they can be built without builing the `check' target. @@ -856,6 +857,9 @@ test_fakehttp_LDADD = $(LDADD) $(POSTLINK) test_getcwidth_SOURCES = test/getcwidth.c test_getcwidth_LDADD = $(POSTLINK) +test_runpty_SOURCES = test/runpty.c +test_runpty_LDADD = $(POSTLINK) + ## ## Our rebuild rules for files that aren't built via the normal mechanisms ## diff --git a/test/install-mh/test-version-check b/test/install-mh/test-version-check index dfe6c9b6..2db8565b 100755 --- a/test/install-mh/test-version-check +++ b/test/install-mh/test-version-check @@ -17,51 +17,15 @@ fi setup_test -require_prog script - actual="${MH_TEST_DIR}/test-version-check$$.actual" context="${MH_TEST_DIR}"/Mail/context version="Version: nmh-${MH_VERSION}" cmd="${MH_TEST_DIR}/Mail/cmd" +runpty="${MH_OBJ_DIR}/test/runpty" -#### Hack: especially on multicore/multiprocessor machines, need -#### to allow time for the output file to be closed. If the output -#### file never reaches a size greater than zero after waiting for -#### a short while, let a subsequent check fail to find what it needs. -#### $1: pid to wait for -#### $2: output file that must eventually have non-zero size -wait_for_script() { - wait $1 - - for i in 1 2 3 4 5 6 7 8 9 10; do - test -s "$2" && break || sleep 1 - done -} - - -if script -S /bin/sh 'echo OK' /dev/null 2>&1 | egrep 'OK' >/dev/null; then - #### script(1) uses -S to set the shell that it runs. - use_dash_S=1 -else - cat >"${cmd}" </dev/null & - wait_for_script $! "${actual}") - if grep OK "${actual}" >/dev/null; then - #### script(1) supports SHELL environment variable. - use_dash_S=0 - else - test_skip "can't find mechanism to set SHELL for script(1)" - fi -fi - -#### Run a command but don't wait for user input. script(1) seems to do -#### what we want by not waiting when run in the background. But check -#### that it makes the command look like it's connected to a terminal below. +#### Run a command but don't wait for user input. We switched from script(1) +#### to our own utility, runpty. Check that it makes the command look like +#### it's connected to a terminal below. run_without_input() { #### Create a command to use as a shell for script. cat >"${cmd}" </dev/null & - else - SHELL="${cmd}" script "${actual}" >/dev/null & - fi - - wait_for_script $! "${actual}" + "$runpty" "${actual}" "${cmd}" } -#### Ensure that script(1) makes the program look like it's connected to a +#### Ensure that runpty makes the program look like it's connected to a #### terminal. The welcome message code in sbr/utils.c needs that. run_without_input 'if test -t 0 -a -t 1 -a -t 2; then echo tty; fi' -grep tty "${actual}" >/dev/null || test_skip "script(1) doesn't simulate tty" +grep tty "${actual}" >/dev/null || test_skip "runpty doesn't simulate tty" # Removing Version will trigger the welcome message. (setup_test diff --git a/test/runpty.c b/test/runpty.c new file mode 100644 index 00000000..a2fe6f48 --- /dev/null +++ b/test/runpty.c @@ -0,0 +1,175 @@ +/* + * runpty.c - Run a process under a pseudo-tty + * + * This code is Copyright (c) 2017, by the authors of nmh. See the + * COPYRIGHT file in the root directory of the nmh distribution for + * complete copyright information. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define COMMAND_TIMEOUT 30 + +int +main(int argc, char *argv[]) +{ + int master, slave, cc, sendeof = 0, status; + time_t starttime; + const char *slavename; + pid_t child; + unsigned char readbuf[1024]; + FILE *output; + + if (argc < 3) { + fprintf(stderr, "Usage: %s output-filename command [arguments ...]\n", + argv[0]); + exit(1); + } + + if ((master = posix_openpt(O_RDWR | O_NOCTTY)) < 0) { + fprintf(stderr, "Unable to open master pseudo-tty: %s\n", + strerror(errno)); + exit(1); + } + + if (grantpt(master) < 0) { + fprintf(stderr, "Unable to grant permissions to master pty: %s\n", + strerror(errno)); + exit(1); + } + + if (unlockpt(master) < 0) { + fprintf(stderr, "Unable to unlock master pty: %s\n", strerror(errno)); + exit(1); + } + + if (!(slavename = ptsname(master))) { + fprintf(stderr, "Unable to determine name of slave pty: %s\n", + strerror(errno)); + exit(1); + } + + if ((slave = open(slavename, O_RDWR | O_NOCTTY)) < 0) { + fprintf(stderr, "Unable to open slave pty \"%s\": %s\n", slavename, + strerror(errno)); + exit(1); + } + + child = fork(); + + /* + * Start the child process if we are in the child; close the master + * as we are supposed to only use the slave ptys here. + */ + + if (child == 0) { + close(master); + dup2(slave, STDIN_FILENO); + dup2(slave, STDOUT_FILENO); + dup2(slave, STDERR_FILENO); + close(slave); + + execvp(argv[2], argv + 2); + + fprintf(stderr, "execvp(%s) failed: %s\n", argv[2], strerror(errno)); + } else if (child < 0) { + fprintf(stderr, "fork() failed: %s\n", strerror(errno)); + exit(1); + } + + close(slave); + + if (!(output = fopen(argv[1], "w"))) { + fprintf(stderr, "Unable to open \"%s\" for output: %s\n", argv[1], + strerror(errno)); + exit(1); + } + + starttime = time(NULL); + + for (;;) { + fd_set readfds; + struct timeval tv; + + FD_ZERO(&readfds); + + FD_SET(master, &readfds); + + /* + * Okay, what's going on here? + * + * We want to send the EOF character (to simulate a "end of file", + * as if we redirected stdin to /dev/null). We can't just close + * master, because we won't be able to get any data back. So, + * after we get SOME data, set sendeof, and that will cause us to + * send the EOF character after the first select call. If we are + * doing sendeof, set the timeout to 1 second; otherwise, set the + * timeout to COMMAND_TIMEOUT seconds remaining. + */ + + if (sendeof) { + tv.tv_sec = 1; + } else { + tv.tv_sec = starttime + COMMAND_TIMEOUT - time(NULL); + if (tv.tv_sec < 0) + tv.tv_sec = 0; + } + tv.tv_usec = 0; + + cc = select(master + 1, &readfds, NULL, NULL, &tv); + + if (cc < 0) { + fprintf(stderr, "select() failed: %s\n", strerror(errno)); + exit(1); + } + + if (cc > 0 && FD_ISSET(master, &readfds)) { + cc = read(master, readbuf, sizeof(readbuf)); + + if (cc < 0) { + fprintf(stderr, "read() failed: %s\n", strerror(errno)); + exit(1); + } + + if (cc == 0) + break; + + fwrite(readbuf, 1, cc, output); + } + + if (cc == 0 && sendeof) { + struct termios rtty; + + if (tcgetattr(master, &rtty) < 0) { + fprintf(stderr, "tcgetattr() failed: %s\n", strerror(errno)); + exit(1); + } + + if (rtty.c_lflag & ICANON) + write(master, &rtty.c_cc[VEOF], 1); + } else if (!sendeof) + sendeof = 1; + + if (time(NULL) > starttime + COMMAND_TIMEOUT) { + fprintf(stderr, "Command execution timed out\n"); + break; + } + } + + close(master); + fclose(output); + + waitpid(child, &status, 0); + + exit(0); +} -- 2.56.0