Mercurial > hg > octave-lojdl > gnulib-hg
changeset 11723:928845623bf5
test-pipe: make a bit more robust.
* tests/test-pipe.c (myerr): Allow error messages regardless of
what we do to stderr.
(test_pipe): Rearrange to avoid deadlock.
(child_main): Try a larger read, to ensure we avoided deadlock.
* lib/pipe.c (create_pipe) [_WIN32]: Fix comment.
* lib/pipe.h (create_pipe_bidi): Document potential for deadlock
if misused.
Signed-off-by: Eric Blake <ebb9@byu.net>
author | Eric Blake <ebb9@byu.net> |
---|---|
date | Mon, 20 Jul 2009 06:41:01 -0600 |
parents | 92457e8bc574 |
children | 99b6167ab3d5 |
files | ChangeLog lib/pipe.c lib/pipe.h tests/test-pipe.c |
diffstat | 4 files changed, 52 insertions(+), 14 deletions(-) [+] |
line wrap: on
line diff
--- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +2009-07-20 Eric Blake <ebb9@byu.net> + + test-pipe: make a bit more robust. + * tests/test-pipe.c (myerr): Allow error messages regardless of + what we do to stderr. + (test_pipe): Rearrange to avoid deadlock. + (child_main): Try a larger read, to ensure we avoided deadlock. + * lib/pipe.c (create_pipe) [_WIN32]: Fix comment. + * lib/pipe.h (create_pipe_bidi): Document potential for deadlock + if misused. + 2009-07-19 Jim Meyering <meyering@redhat.com> fts: avoid false-positive cycle-detection
--- a/lib/pipe.c +++ b/lib/pipe.c @@ -185,9 +185,7 @@ && close (stdoutfd) >= 0))))) /* The child process doesn't inherit ifd[0], ifd[1], ofd[0], ofd[1], but it inherits all open()ed or dup2()ed file handles (which is what - we want in the case of STD*_FILENO) and also orig_stdin, - orig_stdout, orig_stderr (which is not explicitly wanted but - harmless). */ + we want in the case of STD*_FILENO). */ /* Use spawnvpe and pass the environment explicitly. This is needed if the program has modified the environment using putenv() or [un]setenv(). On Windows, programs have two environments, one in the "environment
--- a/lib/pipe.h +++ b/lib/pipe.h @@ -109,6 +109,18 @@ * * Note: When writing to a child process, it is useful to ignore the SIGPIPE * signal and the EPIPE error code. + * + * Note: The parent process must be careful to avoid deadlock. + * 1) If you write more than PIPE_MAX bytes or, more generally, if you write + * more bytes than the subprocess can handle at once, the subprocess + * may write its data and wait on you to read it, but you are currently + * busy writing. + * 2) When you don't know ahead of time how many bytes the subprocess + * will produce, the usual technique of calling read (fd, buf, BUFSIZ) + * with a fixed BUFSIZ will, on Linux 2.2.17 and on BSD systems, cause + * the read() call to block until *all* of the buffer has been filled. + * But the subprocess cannot produce more data until you gave it more + * input. But you are currently busy reading from it. */ extern pid_t create_pipe_bidi (const char *progname, const char *prog_path, char **prog_argv,
--- a/tests/test-pipe.c +++ b/tests/test-pipe.c @@ -34,15 +34,19 @@ #include <string.h> /* Depending on arguments, this test intentionally closes stderr or - starts life with stderr closed. So, the error messages might not - always print, but at least the exit status will be reliable. */ + starts life with stderr closed. So, we arrange to have fd 10 + (outside the range of interesting fd's during the test) set up to + duplicate the original stderr. */ + +static FILE *myerr; + #define ASSERT(expr) \ do \ { \ if (!(expr)) \ { \ - fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ - fflush (stderr); \ + fprintf (myerr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (myerr); \ abort (); \ } \ } \ @@ -52,7 +56,7 @@ static int child_main (int argc, char *argv[]) { - char buffer[1]; + char buffer[2] = { 's', 't' }; int fd; ASSERT (argc == 3); @@ -61,7 +65,7 @@ fd 2 should be closed iff the argument is 1. Check that no other file descriptors leaked. */ - ASSERT (read (STDIN_FILENO, buffer, 1) == 1); + ASSERT (read (STDIN_FILENO, buffer, 2) == 1); buffer[0]++; ASSERT (write (STDOUT_FILENO, buffer, 1) == 1); @@ -141,6 +145,7 @@ /* Push child's input. */ ASSERT (write (fd[1], buffer, 1) == 1); + ASSERT (close (fd[1]) == 0); /* Get child's output. */ ASSERT (read (fd[0], buffer, 2) == 1); @@ -148,7 +153,6 @@ /* Wait for child. */ ASSERT (wait_subprocess (pid, argv0, true, false, true, true, NULL) == 0); ASSERT (close (fd[0]) == 0); - ASSERT (close (fd[1]) == 0); /* Check the result. */ ASSERT (buffer[0] == 'b'); @@ -215,9 +219,22 @@ int main (int argc, char *argv[]) { - ASSERT (argc >= 2); + if (argc < 2) + { + fprintf (stderr, "%s: need arguments\n", argv[0]); + return 2; + } if (strcmp (argv[1], "child") == 0) - return child_main (argc, argv); - else - return parent_main (argc, argv); + { + /* fd 2 might be closed, but fd 10 is the original stderr. */ + myerr = fdopen (10, "w"); + if (!myerr) + return 2; + return child_main (argc, argv); + } + /* We might close fd 2 later, so save it in fd 10. */ + if (dup2 (STDERR_FILENO, 10) != 10 + || (myerr = fdopen (10, "w")) == NULL) + return 2; + return parent_main (argc, argv); }