commit 55a1cb53d6d1a8375afc12679b43901a977cf4b7 Author: Steven Murdoch Steven.Murdoch@cl.cam.ac.uk Date: Fri Jul 22 15:57:56 2011 +0100

Add code to read all from a handle, but this block forever

See http://stackoverflow.com/questions/3722409/windows-child-process-with-redire... for a potential solution --- src/common/util.c | 56 +++++++++++++++++++++++++++++------------------- src/test/test-child.c | 18 +++++++++++++++ src/test/test_util.c | 24 +++++++++++++++++--- 3 files changed, 72 insertions(+), 26 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c index 3d39f59..e9b7909 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3195,6 +3195,7 @@ tor_spawn_background(const char *const filename, const char **argv)

/* Write the error message. GCC requires that we check the return value, but there is nothing we can do if it fails */ + // TODO: Don't use STDOUT, use a pipe set up just for this purpose nbytes = write(STDOUT_FILENO, error_message, error_message_length); nbytes = write(STDOUT_FILENO, hex_errno, sizeof(hex_errno));

@@ -3217,6 +3218,8 @@ tor_spawn_background(const char *const filename, const char **argv) return process_handle; }

+ // TODO: If the child process forked but failed to exec, waitpid it + /* Return read end of the pipes to caller, and close write end */ process_handle.stdout_pipe = stdout_pipe[0]; retval = close(stdout_pipe[1]); @@ -3281,22 +3284,41 @@ tor_get_exit_code(const process_handle_t process_handle) #endif // MS_WINDOWS }

-ssize_t -tor_read_all_from_process_stdout(const process_handle_t process_handle, - char *buf, size_t count) -{ #ifdef MS_WINDOWS +/* Windows equivalent of read_all */ +static ssize_t +read_all_handle(HANDLE h, char *buf, size_t count) +{ + size_t numread = 0; BOOL retval; DWORD bytes_read; - retval = ReadFile(process_handle.stdout_pipe, buf, count, &bytes_read, NULL); - if (!retval) { - log_warn(LD_GENERAL, - "Failed to read from stdin pipe: %s", - format_win32_error(GetLastError())); + + if (count > SIZE_T_CEILING || count > SSIZE_T_MAX) return -1; - } else { - return bytes_read; + + while (numread != count) { + retval = ReadFile(h, buf+numread, count-numread, &bytes_read, NULL); + if (!retval) { + log_warn(LD_GENERAL, + "Failed to read from stdin pipe: %s", + format_win32_error(GetLastError())); + return -1; + } else if (0 == bytes_read) { + /* End of file */ + return bytes_read; + } + numread += bytes_read; } + return (ssize_t)numread; +} +#endif + +ssize_t +tor_read_all_from_process_stdout(const process_handle_t process_handle, + char *buf, size_t count) +{ +#ifdef MS_WINDOWS + return read_all_handle(process_handle.stdout_pipe, buf, count); #else return read_all(process_handle.stdout_pipe, buf, count, 0); #endif @@ -3307,17 +3329,7 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, char *buf, size_t count) { #ifdef MS_WINDOWS - BOOL retval; - DWORD bytes_read; - retval = ReadFile(process_handle.stderr_pipe, buf, count, &bytes_read, NULL); - if (!retval) { - log_warn(LD_GENERAL, - "Failed to read from stderr pipe: %s", - format_win32_error(GetLastError())); - return -1; - } else { - return bytes_read; - } + return read_all_handle(process_handle.stderr_pipe, buf, count); #else return read_all(process_handle.stderr_pipe, buf, count, 0); #endif diff --git a/src/test/test-child.c b/src/test/test-child.c index ca52750..cf49c5d 100644 --- a/src/test/test-child.c +++ b/src/test/test-child.c @@ -1,4 +1,11 @@ #include <stdio.h> +#include "orconfig.h" +#ifdef MS_WINDOWS +#define WINDOWS_LEAN_AND_MEAN +#include <windows.h> +#else +#include <unistd.h> +#endif

/** Trivial test program which prints out its command line arguments so we can * check if tor_spawn_background() works */ @@ -11,7 +18,18 @@ main(int argc, char **argv) fprintf(stderr, "ERR\n"); for (i = 1; i < argc; i++) fprintf(stdout, "%s\n", argv[i]); + fprintf(stdout, "SLEEPING\n"); +#ifdef MS_WINDOWS + Sleep(1000); +#else + sleep(1); +#endif fprintf(stdout, "DONE\n"); +#ifdef MS_WINDOWS + Sleep(1000); +#else + sleep(1); +#endif

return 0; } diff --git a/src/test/test_util.c b/src/test/test_util.c index 28030b7..63c8ad8 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1379,7 +1379,8 @@ test_util_fgets_eagain(void *ptr) /** Helper function for testing tor_spawn_background */ static void run_util_spawn_background(const char *argv[], const char *expected_out, - const char *expected_err, int expected_exit) + const char *expected_err, int expected_exit, + int expected_status) { int retval; ssize_t pos; @@ -1389,7 +1390,12 @@ run_util_spawn_background(const char *argv[], const char *expected_out, /* Start the program */ process_handle = tor_spawn_background(argv[0], argv);

- tt_int_op(process_handle.status, ==, 0); + tt_int_op(process_handle.status, ==, expected_status); + + /* If the process failed to start, don't bother continuing */ + if (process_handle.status == -1) + return; + tt_int_op(process_handle.stdout_pipe, >, 0); tt_int_op(process_handle.stderr_pipe, >, 0);

@@ -1435,21 +1441,31 @@ test_util_spawn_background_ok(void *ptr)

(void)ptr;

- run_util_spawn_background(argv, expected_out, expected_err, 0); + run_util_spawn_background(argv, expected_out, expected_err, 0, 0); }

/** Check that failing to find the executable works as expected */ static void test_util_spawn_background_fail(void *ptr) { +#ifdef MS_WINDOWS const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL}; const char *expected_out = "ERR: Failed to spawn background process " "- code 9/2\n"; const char *expected_err = ""; + const int expected_status = -1; +#else + const char *argv[] = {BUILDDIR "/src/test/no-such-file", "--test", NULL}; + const char *expected_out = "ERR: Failed to spawn background process " + "- code 9/2\n"; + const char *expected_err = ""; + // TODO: Once we can signal failure to exec, set this to be -1; + const int expected_status = 0; +#endif

(void)ptr;

- run_util_spawn_background(argv, expected_out, expected_err, 255); + run_util_spawn_background(argv, expected_out, expected_err, 255, expected_status); }

static void