diff options
author | Jeremy Kerr <jk@ozlabs.org> | 2014-02-20 09:22:58 +0800 |
---|---|---|
committer | Jeremy Kerr <jk@ozlabs.org> | 2014-02-20 10:11:52 +0800 |
commit | ffb84fc1bf1d138d1ac1bb8ece0b7d625f288d97 (patch) | |
tree | fcb5a469816b4b929686f343735a2b2942cf7cf1 | |
parent | 0b3bfb90a292ceba33b649681876ec1ed50c8447 (diff) | |
download | petitboot-ffb84fc1bf1d138d1ac1bb8ece0b7d625f288d97.zip petitboot-ffb84fc1bf1d138d1ac1bb8ece0b7d625f288d97.tar.gz |
lib/process: Don't abort stdout reads on EINTR
If our read() of the process stdout pipe fails with EINTR (eg, if we
receive a SIGCHLD because the process exited), then
process_read_stdout_once will return a non-zero exit code, and we'll
abort any further stdout collection.
Instead, we should check for EINTR, and allow the reads to continue.
This change normalises the return value from process_read_stdout_once to
return positive on success, negative on failure, and zero on competion.
We use a positive return value for the non-error EINTR case.
Also, add a pb_log if the read fails for non-EINTR reasons.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
-rw-r--r-- | lib/process/process.c | 18 | ||||
-rw-r--r-- | test/lib/Makefile.am | 1 | ||||
-rw-r--r-- | test/lib/test-process-stdout-eintr.c | 57 |
3 files changed, 73 insertions, 3 deletions
diff --git a/lib/process/process.c b/lib/process/process.c index 86c7fbb..081a48c 100644 --- a/lib/process/process.c +++ b/lib/process/process.c @@ -61,7 +61,13 @@ static struct process_info *get_info(struct process *process) } /* Read as much as possible into the currently-allocated stdout buffer, and - * possibly realloc it for the next read */ + * possibly realloc it for the next read + * + * Returns: + * > 0 on success (even though no bytes may have been read) + * 0 on EOF (no error, but no more reads can be performed) + * < 0 on error + **/ static int process_read_stdout_once(struct process_info *procinfo) { struct process *process = &procinfo->process; @@ -74,8 +80,14 @@ static int process_read_stdout_once(struct process_info *procinfo) max_len = procinfo->stdout_buf_len - process->stdout_len - 1; rc = read(fd, process->stdout_buf + process->stdout_len, max_len); - if (rc <= 0) + if (rc == 0) + return 0; + if (rc < 0) { + if (errno == EINTR) + return 1; + pb_log("%s: read failed: %s\n", __func__, strerror(errno)); return rc; + } process->stdout_len += rc; if (process->stdout_len == procinfo->stdout_buf_len - 1) { @@ -85,7 +97,7 @@ static int process_read_stdout_once(struct process_info *procinfo) procinfo->stdout_buf_len); } - return rc; + return 1; } static int process_setup_stdout_pipe(struct process_info *procinfo) diff --git a/test/lib/Makefile.am b/test/lib/Makefile.am index ed570af..23bee36 100644 --- a/test/lib/Makefile.am +++ b/test/lib/Makefile.am @@ -32,6 +32,7 @@ check_PROGRAMS = list-test \ test-process-async-stdout \ test-process-parent-stdout \ test-process-both \ + test-process-stdout-eintr \ test-fold TESTS = $(check_PROGRAMS) diff --git a/test/lib/test-process-stdout-eintr.c b/test/lib/test-process-stdout-eintr.c new file mode 100644 index 0000000..b62d570 --- /dev/null +++ b/test/lib/test-process-stdout-eintr.c @@ -0,0 +1,57 @@ + +#include <stdlib.h> +#include <string.h> +#include <assert.h> +#include <signal.h> + +#include <process/process.h> +#include <waiter/waiter.h> +#include <talloc/talloc.h> + +static int do_child(int ppid) +{ + sleep(1); + kill(ppid, SIGCHLD); + printf("forty two\n"); + return 42; +} + +int main(int argc, char **argv) +{ + struct waitset *waitset; + struct process *process; + const char *child_argv[3]; + void *ctx; + + if (argc == 3 && !strcmp(argv[1], "child")) + return do_child(atoi(argv[2])); + + ctx = talloc_new(NULL); + + waitset = waitset_create(ctx); + + process_init(ctx, waitset, false); + + child_argv[0] = argv[0]; + child_argv[1] = "child"; + child_argv[2] = talloc_asprintf(ctx, "%d", getpid()); + child_argv[3] = NULL; + + process = process_create(ctx); + process->path = child_argv[0]; + process->argv = child_argv; + process->keep_stdout = true; + + process_run_sync(process); + + assert(WIFEXITED(process->exit_status)); + assert(WEXITSTATUS(process->exit_status) == 42); + + assert(process->stdout_len == strlen("forty two\n")); + assert(!memcmp(process->stdout_buf, "forty two\n", + process->stdout_len)); + + talloc_free(ctx); + + return EXIT_SUCCESS; +} |