Hi Mattias Just some comments below (please ignore the mangled formatting.)
On Sat, Jul 7, 2018 at 11:26 PM, Mattias Andrée <[email protected]> wrote: > Signed-off-by: Mattias Andrée <[email protected]> > --- > Makefile | 20 +- > test-common.c | 823 > ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > test-common.h | 190 ++++++++++++++ > tty.test.c | 26 ++ > 4 files changed, 1055 insertions(+), 4 deletions(-) > create mode 100644 test-common.c > create mode 100644 test-common.h > create mode 100644 tty.test.c > > diff --git a/Makefile b/Makefile > index 0e421e7..005cf13 100644 > --- a/Makefile > +++ b/Makefile > @@ -1,7 +1,7 @@ > include config.mk > > .SUFFIXES: > -.SUFFIXES: .o .c > +.SUFFIXES: .test .test.o .o .c > > HDR =\ > arg.h\ > @@ -19,7 +19,8 @@ HDR =\ > sha512-256.h\ > text.h\ > utf.h\ > - util.h > + util.h\ > + test-common.h > > LIBUTF = libutf.a > LIBUTFSRC =\ > @@ -181,9 +182,12 @@ BIN =\ > xinstall\ > yes > > +TEST =\ > + tty.test > + > LIBUTFOBJ = $(LIBUTFSRC:.c=.o) > LIBUTILOBJ = $(LIBUTILSRC:.c=.o) > -OBJ = $(BIN:=.o) $(LIBUTFOBJ) $(LIBUTILOBJ) > +OBJ = $(BIN:=.o) $(TEST:=.o) test-common.o $(LIBUTFOBJ) $(LIBUTILOBJ) > SRC = $(BIN:=.c) > MAN = $(BIN:=.1) > > @@ -193,12 +197,17 @@ $(BIN): $(LIB) $(@:=.o) > > $(OBJ): $(HDR) config.mk > > +$(TEST): $(@:=.o) test-common.o > + > .o: > $(CC) $(LDFLAGS) -o $@ $< $(LIB) > > .c.o: > $(CC) $(CFLAGS) $(CPPFLAGS) -o $@ -c $< > > +.test.o.test: > + $(CC) $(LDFLAGS) -o $@ $< test-common.o > + > $(LIBUTF): $(LIBUTFOBJ) > $(AR) rc $@ $? > $(RANLIB) $@ > @@ -212,6 +221,9 @@ getconf.o: getconf.h > getconf.h: getconf.sh > ./getconf.sh > $@ > > +check: $(TEST) $(BIN) > + @set -e; for f in $(TEST); do echo ./$$f; ./$$f; done > + > install: all > mkdir -p $(DESTDIR)$(PREFIX)/bin > cp -f $(BIN) $(DESTDIR)$(PREFIX)/bin > @@ -271,7 +283,7 @@ sbase-box-uninstall: uninstall > cd $(DESTDIR)$(PREFIX)/bin && rm -f sbase-box > > clean: > - rm -f $(BIN) $(OBJ) $(LIB) sbase-box sbase-$(VERSION).tar.gz > + rm -f $(BIN) $(TEST) $(OBJ) $(LIB) sbase-box sbase-$(VERSION).tar.gz > rm -f getconf.h > > .PHONY: all install uninstall dist sbase-box sbase-box-install > sbase-box-uninstall clean > diff --git a/test-common.c b/test-common.c > new file mode 100644 > index 0000000..458b094 > --- /dev/null > +++ b/test-common.c > @@ -0,0 +1,823 @@ > +/* See LICENSE file for copyright and license details. */ > +#include "test-common.h" > + > +struct Counter { > + const char *name; > + size_t value; > +}; > + > +const char *test_file = NULL; > +int test_line = 0; > +int main_ret = 0; > +int timeout = 10; > +int pdeath_sig = SIGINT; > +void (*atfork)(void) = NULL; > + > +static struct Counter counters[16]; > +static size_t ncounters = 0; > +static pid_t async_pids[1024]; > +static size_t async_npids = 0; > + > +static void > +eperror(const char *prefix) > +{ > + perror(prefix); > + fflush(stderr); > + exit(64); > +} > + > +struct Process * > +stdin_text(struct Process *proc, char *s) > +{ > + proc->input[0].data = s; > + proc->input[0].flags &= ~IO_STREAM_BINARY; > + return proc; > +} > + > +struct Process * > +stdin_bin(struct Process *proc, char *s, size_t n) > +{ > + proc->input[0].data = s; > + proc->input[0].len = n; > + proc->input[0].flags |= IO_STREAM_BINARY; > + return proc; > +} > + > +struct Process * > +stdin_fds(struct Process *proc, int input_fd, int output_fd) > +{ > + proc->input[0].flags &= ~IO_STREAM_DATA; > + proc->input[0].input_fd = input_fd; > + proc->input[0].output_fd = output_fd; > + return proc; > +} > + > +struct Process * > +stdin_type(struct Process *proc, int type) > +{ > + proc->input[0].flags &= ~IO_STREAM_CREATE_MASK; > + proc->input[0].flags |= type; > + return proc; > +} > + > +struct Process * > +stdout_fds(struct Process *proc, int input_fd, int output_fd) > +{ > + proc->output[0].flags &= ~IO_STREAM_DATA; > + proc->output[0].input_fd = input_fd; > + proc->output[0].output_fd = output_fd; > + return proc; > +} > + > +struct Process * > +stdout_type(struct Process *proc, int type) > +{ > + proc->output[0].flags &= ~IO_STREAM_CREATE_MASK; > + proc->output[0].flags |= type; > + return proc; > +} > + > +struct Process * > +stderr_fds(struct Process *proc, int input_fd, int output_fd) > +{ > + proc->output[1].flags &= ~IO_STREAM_DATA; > + proc->output[1].input_fd = input_fd; > + proc->output[1].output_fd = output_fd; > + return proc; > +} > + > +struct Process * > +stderr_type(struct Process *proc, int type) > +{ > + proc->output[1].flags &= ~IO_STREAM_CREATE_MASK; > + proc->output[1].flags |= type; > + return proc; > +} > + > +struct Process * > +set_preexec(struct Process *proc, void (*preexec)(struct Process *)) > +{ > + proc->preexec = preexec; > + return proc; > +} > + > +struct Process * > +set_async(struct Process *proc) > +{ > + proc->flags |= PROCESS_ASYNC; > + return proc; > +} > + > +struct Process * > +set_setsid(struct Process *proc) > +{ > + proc->flags |= PROCESS_SETSID; > + return proc; > +} > + > +void > +push_counter(const char *name) > +{ > + if (ncounters == ELEMSOF(counters)) { > + fprintf(stderr, "too many counters\n"); > + fflush(stderr); > + exit(64); > + } > + counters[ncounters++].name = name; > +} > + > +void > +set_counter(size_t value) > +{ > + if (!ncounters) { > + fprintf(stderr, "no counters have been pushed\n"); > + fflush(stderr); > + exit(64); > + } > + counters[ncounters - 1].value = value; > +} > + > +void > +pop_counter(void) > +{ > + if (!ncounters) { > + fprintf(stderr, "all counters have already been popped\n"); > + fflush(stderr); > + exit(64); > + } > + ncounters--; > +} > + > +pid_t > +async_fork(int *retp) > +{ > + pid_t pid; > + > + if (!retp) > + retp = &(int){0}; > + > + if (async_npids == ELEMSOF(async_pids)) > + *retp = async_join(); > + > + switch ((pid = fork())) { > + case -1: > + eperror("fork"); > + case 0: > + if (atfork) > + atfork(); > + async_npids = 0; > +#ifdef PR_SET_PDEATHSIG > + prctl(PR_SET_PDEATHSIG, pdeath_sig); > +#endif > + alarm(timeout); > + break; > + default: > + async_pids[async_npids++] = pid; > + break; > + } > + > + return pid; > +} > + > +int > +async_join(void) > +{ > + int ret = 0, status; > + > + while (async_npids--) { > + if (waitpid(async_pids[async_npids], &status, 0) != async_pids[async_npids]) > + eperror("waitpid"); > + ret |= WIFEXITED(status) ? WEXITSTATUS(status) : 64; > + } > + > + main_ret |= status; > + async_npids = 0; > + return ret; > +} > + > +void > +start_process(struct Process *proc) > +{ > + struct InputStream *in; > + struct OutputStream *out; > + int exec_sig_pipe[2]; > + int fd, minfd = 3, fds[2], status; > + size_t i; > + ssize_t r; > + pid_t pid; > + > + if (!proc->file) > + proc->file = proc->argv[0]; > + > + for (i = 0; i < proc->ninput; i++) > + if (minfd <= proc->input[i].fd) > + minfd = proc->input[i].fd + 1; > + for (i = 0; i < proc->noutput; i++) > + if (minfd <= proc->output[i].fd) > + minfd = proc->output[i].fd + 1; > + > + if (pipe(exec_sig_pipe)) > + eperror("pipe"); > + > + for (i = 0; i < 2; i++) { > + fd = fcntl(exec_sig_pipe[i], F_DUPFD_CLOEXEC, minfd); > + if (fd < 0) > + eperror("fcntl F_DUPFD_CLOEXEC"); > + close(exec_sig_pipe[i]); > + exec_sig_pipe[i] = fd; > + } > + > + for (i = 0; i < proc->ninput; i++) { > + in = &proc->input[i]; > + in->name = NULL; > + if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_PIPE) { > + if (pipe(fds)) > + eperror("pipe"); > + } else if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_SOCK_STREAM) { > + if (socketpair(PF_LOCAL, SOCK_STREAM, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_STREAM"); > + } else if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_SOCK_DGRAM) { > + if (socketpair(PF_LOCAL, SOCK_DGRAM, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_DGRAM"); > + } else if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_SOCK_SEQPACKET) > { > + if (socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_SEQPACKET"); > + } else if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_TTY_NOCTTY) { > + in->name = strdup(openpt(0, fds)); > + if (!in->name) > + eperror("strdup"); > + if (!(in->flags & IO_STREAM_MASTER)) > + fd = fds[0], fds[0] = fds[1], fds[1] = fd; > + } else if ((in->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_TTY_CTTY) { Instead of repeating (in->flags & IO_STREAM_CREATE_MASK) 5 times just setting it once like int createmask = in->flags & IO_STREAM_CREATE_MASK and then using "createmask == etc." looks cleaner. > + in->name = strdup(openpt(1, fds)); > + if (!in->name) > + eperror("strdup"); > + if (!(in->flags & IO_STREAM_MASTER)) > + fd = fds[0], fds[0] = fds[1], fds[1] = fd; > + } else { > + goto precreated_input; > + } > + in->input_fd = fds[0]; > + in->output_fd = fds[1]; > + precreated_input: > + if (in->flags & IO_STREAM_DATA) { > + switch (pid = fork()) { > + case -1: > + eperror("fork"); > + case 0: > + break; > + default: > + if (waitpid(pid, &status, 0) != pid) > + eperror("waitpid"); > + if (status) > + exit(64); > + if (!(in->flags & IO_STREAM_KEEP_OPEN)) > + close(in->output_fd); > + continue; > + } > + if (atfork) > + atfork(); > + dealloc_process(proc); > + close(in->input_fd); > + close(exec_sig_pipe[0]); > + close(exec_sig_pipe[1]); > + while (i--) { > + if (proc->input[i].input_fd > 0) > + close(proc->input[i].input_fd); > + if (proc->input[i].output_fd > 0) > + close(proc->input[i].output_fd); > + } > + switch (pid = fork()) { > + case -1: > + eperror("fork"); > + case 0: > + exit(0); > + default: > + break; > + } > + while (in->len) { > + r = write(in->output_fd, in->data, in->len); > + if (r < 0) > + eperror("write"); > + in->data += r; > + in->len -= (size_t)r; > + } > + close(in->output_fd); > + exit(0); > + } > + } > + > + for (i = 0; i < proc->noutput; i++) { > + out = &proc->output[i]; > + out->name = NULL; > + if ((out->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_PIPE) { > + if (pipe(fds)) > + eperror("pipe"); > + } else if ((out->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_SOCK_STREAM) { > + if (socketpair(PF_LOCAL, SOCK_STREAM, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_STREAM"); > + } else if ((out->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_SOCK_DGRAM) { > + if (socketpair(PF_LOCAL, SOCK_DGRAM, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_DGRAM"); > + } else if ((out->flags & IO_STREAM_CREATE_MASK) == > IO_STREAM_SOCK_SEQPACKET) { > + if (socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, fds)) > + eperror("socketpair PF_LOCAL SOCK_SEQPACKET"); > + } else if ((out->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_TTY_NOCTTY) { > + out->name = strdup(openpt(0, fds)); > + if (!out->name) > + eperror("strdup"); > + if (out->flags & IO_STREAM_MASTER) > + fd = fds[0], fds[0] = fds[1], fds[1] = fd; > + } else if ((out->flags & IO_STREAM_CREATE_MASK) == IO_STREAM_TTY_CTTY) { > + out->name = strdup(openpt(1, fds)); > + if (!out->name) > + eperror("strdup"); > + if (out->flags & IO_STREAM_MASTER) > + fd = fds[0], fds[0] = fds[1], fds[1] = fd; > + } else { > + continue; > + } > + out->input_fd = fds[0]; > + out->output_fd = fds[1]; > + } > + > + switch ((proc->pid = fork())) { > + case -1: > + eperror("fork"); > + case 0: > + break; > + default: > + if (atfork) > + atfork(); > + for (i = 0; i < proc->ninput; i++) > + close(proc->input[i].input_fd); > + for (i = 0; i < proc->noutput; i++) > + close(proc->output[i].output_fd); > + close(exec_sig_pipe[1]); > + read(exec_sig_pipe[0], &(char){0}, 1); > + if (clock_gettime(CLOCK_MONOTONIC, &proc->start_time)) > + eperror("clock_gettime CLOCK_MONOTONIC"); > + close(exec_sig_pipe[0]); > + return; > + } > + > +#ifdef PR_SET_PDEATHSIG > + prctl(PR_SET_PDEATHSIG, proc->pdeath_sig); > +#endif > + > + for (i = 0; i < proc->ninput; i++) { > + fd = fcntl(proc->input[i].input_fd, F_DUPFD, minfd); > + if (fd < 0) > + eperror("fcntl F_DUPFD"); Would be nice to know if it failed on the input or output side: eperror("input fcntl F_DUPFD"); and eperror("output fcntl F_DUPFD"); respectively. > + close(proc->input[i].input_fd); > + proc->input[i].input_fd = fd; > + } > + > + for (i = 0; i < proc->noutput; i++) { > + fd = fcntl(proc->output[i].output_fd, F_DUPFD, minfd); > + if (fd < 0) > + eperror("fcntl F_DUPFD"); See above. > + close(proc->output[i].output_fd); > + proc->output[i].output_fd = fd; > + } > + > + for (i = 0; i < proc->ninput; i++) { > + if (dup2(proc->input[i].input_fd, proc->input[i].fd) != proc->input[i].fd) > + eperror("dup2"); See above. > + close(proc->input[i].input_fd); > + } > + > + for (i = 0; i < proc->noutput; i++) { > + if (dup2(proc->output[i].output_fd, proc->output[i].fd) != > proc->output[i].fd) > + eperror("dup2"); See above. I will have a closer look at everything after you have slimmed down the test-common.{c,h} (as discussed). Cheers, Silvan
