August 2011 - tor-commits - lists.torproject.org

[tor/master] Fix compilation on non-Windows platforms
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 99baa7e45cc65f0683912c59934679df554fa306 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Sat Jul 23 14:32:39 2011 +0100 Fix compilation on non-Windows platforms --- src/common/util.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 2691b1f..e63047b 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3094,6 +3094,11 @@ tor_spawn_background(const char *const filename, const char **argv) static int max_fd = -1; + // XXX + process_handle.pid = 0; + process_handle.stderr_pipe = 0; + process_handle.stdout_pipe = 0; + /* We do the strlen here because strlen() is not signal handler safe, and we are not allowed to use unsafe functions between fork and exec */ error_message_length = strlen(error_message); @@ -3267,11 +3272,12 @@ tor_get_exit_code(const process_handle_t process_handle) } #else int stat_loc; + int retval; retval = waitpid(process_handle.pid, &stat_loc, 0); if (retval != process_handle.pid) { log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid, - sterror(errno)); + strerror(errno)); return -1; } @@ -3457,7 +3463,9 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, /* When fw-helper fails, how long do we wait until running it again */ #define TIME_TO_EXEC_FWHELPER_FAIL 60 + // XXX: remove static int child_pid = -1; + static process_handle_t child_handle = {0, 0, 0, 0}; static FILE *stdout_read = NULL; static FILE *stderr_read = NULL; static time_t time_to_run_helper = 0; @@ -3491,7 +3499,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, /* Assume tor-fw-helper will succeed, start it later*/ time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS; - child_pid = tor_spawn_background(filename, &fd_out, &fd_err, argv); + child_handle = tor_spawn_background(filename, argv); if (child_pid < 0) { log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", filename);

1 0

[tor/master] If hProcess is NULL, read_all_handle returns if it would block
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit c5796a8fb2fc70085c771d6296d4f29a7ddb68ae Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Sat Jul 23 21:35:50 2011 +0100 If hProcess is NULL, read_all_handle returns if it would block --- src/common/util.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 31d5586..2691b1f 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3304,12 +3304,12 @@ read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) "Failed to peek from handle: %s", format_win32_error(GetLastError())); return -1; - } else if (0 == byte_count) { + } else if (0 == byte_count) /* Nothing available: process exited or it is busy */ - /* Keep on reading if we don't know whether the process is running */ + /* Exit if we don't know whether the process is running */ if (NULL == hProcess) - continue; + break; /* The process exited and there's nothing left to read from it */ if (process_exited)

1 0

[tor/master] Complete logging of output from port forwarding helper
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 7d015c886a3fc985ab767de01811d1591f0ac86a Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Aug 18 18:41:23 2011 +0100 Complete logging of output from port forwarding helper --- src/common/util.c | 139 +++++++++++++++++++++++++++++++++++++------------- src/common/util.h | 6 ++- src/test/test_util.c | 4 +- 3 files changed, 110 insertions(+), 39 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 5a5f3b5..3769d06 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3070,7 +3070,7 @@ tor_spawn_background(const char *const filename, const char **argv) // TODO: Close hProcess and hThread in process_handle.pid? process_handle.stdout_pipe = stdout_pipe_read; process_handle.stderr_pipe = stderr_pipe_read; - process_handle.status = 0; + process_handle.status = 1; } // TODO: Close pipes on exit @@ -3248,20 +3248,36 @@ tor_spawn_background(const char *const filename, const char **argv) needs to know about the pid in order to reap it later */ } - process_handle.status = 0; + process_handle.status = 1; process_handle.pid = pid; + /* Set stdout/stderr pipes to be non-blocking */ + fcntl(process_handle.stdout_pipe, F_SETFL, O_NONBLOCK); + fcntl(process_handle.stderr_pipe, F_SETFL, O_NONBLOCK); + /* Open the buffered IO streams */ + process_handle.stdout_handle = fdopen(process_handle.stdout_pipe, "r"); + process_handle.stderr_handle = fdopen(process_handle.stderr_pipe, "r"); + return process_handle; #endif // MS_WINDOWS } int -tor_get_exit_code(const process_handle_t process_handle) +tor_get_exit_code(const process_handle_t process_handle, int block) { #ifdef MS_WINDOWS DWORD exit_code; BOOL retval; - WaitForSingleObject(process_handle.pid.hProcess, INFINITE); - retval = GetExitCodeProcess(process_handle.pid.hProcess, &exit_code); + if (block) { + exit_code = WaitForSingleObject(process_handle.pid.hProcess, INFINITE); + retval = GetExitCodeProcess(process_handle.pid.hProcess, &exit_code); + } else { + exit_code = WaitForSingleObject(process_handle.pid.hProcess, 0); + if (WAIT_TIMEOUT == exit_code) { + // Process has not exited + return -2; + } + retval = GetExitCodeProcess(process_handle.pid.hProcess, &exit_code); + } if (!retval) { log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s", @@ -3371,6 +3387,55 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, #endif } +#ifdef MS_WINDOWS +static int +log_from_handle(HANDLE *pipe, int severity) +{ + char buf[256]; + int pos; + int start, cur, next; + + pos = tor_read_all_handle(pipe, buf, sizeof(buf) - 1, NULL); + if (pos < 0) { + // Error + log_warn(LD_GENERAL, "Failed to read data from subprocess"); + return -1; + } + + if (0 == pos) { + // There's nothing to read (process is busy or has exited) + log_debug(LD_GENERAL, "Subprocess had nothing to say"); + return 0; + } + + // End with a null even if there isn't a \r\n at the end + // TODO: What if this is a partial line? + buf[pos] = '\0'; + log_debug(LD_GENERAL, "Subprocess had %d bytes to say", pos); + + next = 0; // Start of the next line + while (next < pos) { + start = next; // Look for the end of this line + for (cur=start; cur<pos; cur++) { + if ('\r' == buf[cur]) { + buf[cur] = '\0'; + next = cur + 1; + if ((cur + 1) < pos && buf[cur+1] < pos) { + buf[cur + 1] = '\0'; + next = cur + 2; + } + // Line starts at start and ends with a null (was \r\n) + break; + } + // Line starts at start and ends at the end of a string + // but we already added a null in earlier + } + log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", buf+start); + } + return pos; +} + +#else /** Read from stream, and send lines to log at the specified log level. * Returns 1 if stream is closed normally, -1 if there is a error reading, and * 0 otherwise. Handles lines from tor-fw-helper and @@ -3446,28 +3511,23 @@ log_from_pipe(FILE *stream, int severity, const char *executable, /* We should never get here */ return -1; } +#endif void tor_check_port_forwarding(const char *filename, int dir_port, int or_port, time_t now) { -#ifdef MS_WINDOWS - (void) filename; (void) dir_port; (void) or_port; (void) now; - (void) tor_spawn_background; - (void) log_from_pipe; - log_warn(LD_GENERAL, "Sorry, port forwarding is not yet supported " - "on windows."); -#else /* When fw-helper succeeds, how long do we wait until running it again */ #define TIME_TO_EXEC_FWHELPER_SUCCESS 300 /* When fw-helper fails, how long do we wait until running it again */ #define TIME_TO_EXEC_FWHELPER_FAIL 60 - // XXX: remove - static int child_pid = -1; - static process_handle_t child_handle = {0, 0, 0, 0}; - static FILE *stdout_read = NULL; - static FILE *stderr_read = NULL; +#ifdef MS_WINDOWS + static process_handle_t child_handle = {0, NULL, NULL, {NULL}}; +#else + static process_handle_t child_handle; +#endif + static time_t time_to_run_helper = 0; int stdout_status, stderr_status, retval; const char *argv[10]; @@ -3492,37 +3552,40 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, argv[9] = NULL; /* Start the child, if it is not already running */ - if (-1 == child_pid && - time_to_run_helper < now) { - int fd_out=-1, fd_err=-1; - + if (child_handle.status <= 0 && time_to_run_helper < now) { /* Assume tor-fw-helper will succeed, start it later*/ time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS; child_handle = tor_spawn_background(filename, argv); - if (child_pid < 0) { + if (child_handle.status < 0) { log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", filename); - child_pid = -1; return; } - /* Set stdout/stderr pipes to be non-blocking */ - fcntl(fd_out, F_SETFL, O_NONBLOCK); - fcntl(fd_err, F_SETFL, O_NONBLOCK); - /* Open the buffered IO streams */ - stdout_read = fdopen(fd_out, "r"); - stderr_read = fdopen(fd_err, "r"); - +#ifdef MS_WINDOWS + log_info(LD_GENERAL, + "Started port forwarding helper (%s)", filename); +#else log_info(LD_GENERAL, "Started port forwarding helper (%s) with pid %d", filename, child_pid); +#endif } /* If child is running, read from its stdout and stderr) */ - if (child_pid > 0) { + if (child_handle.status > 0) { /* Read from stdout/stderr and log result */ retval = 0; - stdout_status = log_from_pipe(stdout_read, LOG_INFO, filename, &retval); - stderr_status = log_from_pipe(stderr_read, LOG_WARN, filename, &retval); +#ifdef MS_WINDOWS + stdout_status = log_from_handle(child_handle.stdout_pipe, LOG_INFO); + stderr_status = log_from_handle(child_handle.stderr_pipe, LOG_WARN); + // If we got this far (on Windows), the process started + retval = 0; +#else + stdout_status = log_from_pipe(child_handle.stdout_handle, + LOG_INFO, filename, &retval); + stderr_status = log_from_pipe(child_handle.stderr_handle, + LOG_WARN, filename, &retval); +#endif if (retval) { /* There was a problem in the child process */ time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; @@ -3532,9 +3595,16 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, if (-1 == stdout_status || -1 == stderr_status) /* There was a failure */ retval = -1; +#ifdef MS_WINDOWS + else if (tor_get_exit_code(child_handle, 0) >= 0) { + /* process has exited */ + retval = 1; + } +#else else if (1 == stdout_status || 1 == stderr_status) /* stdout or stderr was closed */ retval = 1; +#endif else /* Both are fine */ retval = 0; @@ -3549,9 +3619,8 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, /* TODO: The child might not actually be finished (maybe it failed or closed stdout/stderr), so maybe we shouldn't start another? */ - child_pid = -1; + child_handle.status = -1; } } -#endif } diff --git a/src/common/util.h b/src/common/util.h index 2efe557..40fe305 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -349,7 +349,7 @@ HANDLE load_windows_system_library(const TCHAR *library_name); /* Prototypes for private functions only used by util.c (and unit tests) */ typedef struct process_handle_s { - int status; + int status; // 0: not running; 1: running; -1: error #ifdef MS_WINDOWS HANDLE stdout_pipe; HANDLE stderr_pipe; @@ -357,13 +357,15 @@ typedef struct process_handle_s { #else int stdout_pipe; int stderr_pipe; + FILE *stdout_handle; + FILE *stderr_handle; int pid; #endif // MS_WINDOWS } process_handle_t; process_handle_t tor_spawn_background(const char *const filename, const char **argv); -int tor_get_exit_code(const process_handle_t pid); +int tor_get_exit_code(const process_handle_t pid, int block); ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess); ssize_t tor_read_all_from_process_stdout(const process_handle_t process_handle, char *buf, size_t count); diff --git a/src/test/test_util.c b/src/test/test_util.c index 071fd04..6f87560 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1408,7 +1408,7 @@ run_util_spawn_background(const char *argv[], const char *expected_out, tt_int_op(pos, ==, strlen(expected_out)); /* Check it terminated correctly */ - retval = tor_get_exit_code(process_handle); + retval = tor_get_exit_code(process_handle, 1); tt_int_op(retval, ==, expected_exit); // TODO: Make test-child exit with something other than 0 @@ -1529,7 +1529,7 @@ test_util_spawn_background_partial_read(void *ptr) #endif /* Check it terminated correctly */ - retval = tor_get_exit_code(process_handle); + retval = tor_get_exit_code(process_handle, 1); tt_int_op(retval, ==, expected_exit); // TODO: Make test-child exit with something other than 0

1 0

[tor/master] Test case for reading the partial output of a background process
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 5bf9890b3b18ad98f5ac601992bc08533ce9e209 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Sun Jul 24 23:31:59 2011 +0100 Test case for reading the partial output of a background process --- src/common/util.c | 14 ++++---- src/common/util.h | 1 + src/test/test-child.c | 4 ++ src/test/test_util.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index e63047b..5a5f3b5 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3292,8 +3292,8 @@ tor_get_exit_code(const process_handle_t process_handle) #ifdef MS_WINDOWS /* Windows equivalent of read_all */ -static ssize_t -read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) +ssize_t +tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) { size_t numread = 0; BOOL retval; @@ -3310,7 +3310,7 @@ read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) "Failed to peek from handle: %s", format_win32_error(GetLastError())); return -1; - } else if (0 == byte_count) + } else if (0 == byte_count) { /* Nothing available: process exited or it is busy */ /* Exit if we don't know whether the process is running */ @@ -3352,8 +3352,8 @@ 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, - process_handle.pid.hProcess); + return tor_read_all_handle(process_handle.stdout_pipe, buf, count, + process_handle.pid.hProcess); #else return read_all(process_handle.stdout_pipe, buf, count, 0); #endif @@ -3364,8 +3364,8 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, char *buf, size_t count) { #ifdef MS_WINDOWS - return read_all_handle(process_handle.stderr_pipe, buf, count, - process_handle.pid.hProcess); + return tor_read_all_handle(process_handle.stderr_pipe, buf, count, + process_handle.pid.hProcess); #else return read_all(process_handle.stderr_pipe, buf, count, 0); #endif diff --git a/src/common/util.h b/src/common/util.h index c111ba7..2efe557 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -364,6 +364,7 @@ typedef struct process_handle_s { process_handle_t tor_spawn_background(const char *const filename, const char **argv); int tor_get_exit_code(const process_handle_t pid); +ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess); ssize_t tor_read_all_from_process_stdout(const process_handle_t process_handle, char *buf, size_t count); ssize_t tor_read_all_from_process_stderr(const process_handle_t process_handle, diff --git a/src/test/test-child.c b/src/test/test-child.c index cf49c5d..1b9c5e3 100644 --- a/src/test/test-child.c +++ b/src/test/test-child.c @@ -19,6 +19,10 @@ main(int argc, char **argv) for (i = 1; i < argc; i++) fprintf(stdout, "%s\n", argv[i]); fprintf(stdout, "SLEEPING\n"); + /* We need to flush stdout so that test_util_spawn_background_partial_read() + succeed. Otherwise ReadFile() will get the entire output in one */ + // XXX: Can we make stdio flush on newline? + fflush(stdout); #ifdef MS_WINDOWS Sleep(1000); #else diff --git a/src/test/test_util.c b/src/test/test_util.c index c6dc98d..071fd04 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1468,6 +1468,83 @@ test_util_spawn_background_fail(void *ptr) run_util_spawn_background(argv, expected_out, expected_err, 255, expected_status); } +/** Helper function for testing tor_spawn_background */ +static void +test_util_spawn_background_partial_read(void *ptr) +{ +#ifdef MS_WINDOWS + // TODO: Under MSYS, BUILDDIR in orconfig.h needs to be tweaked + const char *argv[] = {BUILDDIR "/src/test/test-child.exe", "--test", NULL}; + const char *expected_out[] = { "OUT\r\n--test\r\nSLEEPING\r\n", + "DONE\r\n", + NULL }; + const char *expected_err = "ERR\r\n"; + int expected_out_ctr; +#else + const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; + const char *expected_out = "OUT\n--test\nSLEEPING\nDONE\n"; + const char *expected_err = "ERR\r\n"; +#endif + const int expected_exit = 0; + const int expected_status = 0; + + int retval; + ssize_t pos; + process_handle_t process_handle; + char stdout_buf[100], stderr_buf[100]; + + /* Start the program */ + process_handle = tor_spawn_background(argv[0], argv); + tt_int_op(process_handle.status, ==, expected_status); + + /* Check stdout */ +#ifdef MS_WINDOWS + for (expected_out_ctr =0; expected_out[expected_out_ctr] != NULL;) { + pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, + sizeof(stdout_buf) - 1, NULL); + log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos); + + /* We would have blocked, keep on trying */ + if (0 == pos) + continue; + + tt_assert(pos >= 0); + stdout_buf[pos] = '\0'; + tt_str_op(stdout_buf, ==, expected_out[expected_out_ctr]); + tt_int_op(pos, ==, strlen(expected_out[expected_out_ctr])); + expected_out_ctr++; + } + /* The process should have exited without writing more */ + pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, + sizeof(stdout_buf) - 1, + process_handle.pid.hProcess); + tt_int_op(pos, ==, 0); +#else + pos = tor_read_all_from_process_stdout(process_handle, stdout_buf, + sizeof(stdout_buf) - 1); + tt_assert(pos >= 0); + stdout_buf[pos] = '\0'; + tt_str_op(stdout_buf, ==, expected_out); + tt_int_op(pos, ==, strlen(expected_out)); +#endif + + /* Check it terminated correctly */ + retval = tor_get_exit_code(process_handle); + tt_int_op(retval, ==, expected_exit); + // TODO: Make test-child exit with something other than 0 + + /* Check stderr */ + pos = tor_read_all_from_process_stderr(process_handle, stderr_buf, + sizeof(stderr_buf) - 1); + tt_assert(pos >= 0); + stderr_buf[pos] = '\0'; + tt_str_op(stderr_buf, ==, expected_err); + tt_int_op(pos, ==, strlen(expected_err)); + + done: + ; +} + static void test_util_di_ops(void) { @@ -1555,6 +1632,7 @@ struct testcase_t util_tests[] = { #endif UTIL_TEST(spawn_background_ok, 0), UTIL_TEST(spawn_background_fail, 0), + UTIL_TEST(spawn_background_partial_read, 0), END_OF_TESTCASES };

1 0

[tor/master] Add code to read all from a handle, but this block forever
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 55a1cb53d6d1a8375afc12679b43901a977cf4b7 Author: Steven Murdoch <Steven.Murdoch(a)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-redir… 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

1 0

[tor/master] XXX Hack to allow tor-fw-helper to be built on Windows
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 2eca0165945f4caddf0585f3f9a33e422f1deeb1 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Wed Jul 20 16:07:13 2011 +0100 XXX Hack to allow tor-fw-helper to be built on Windows For some reason, --with-libminiupnpc-dir doesn't work on Windows, so this hardcodes /local/lib as the path in which libminiupnpc.a can be found. Also, libminiupnpc needs libws2_32 and libiphlpapi under Windows, so this hardcodes these libraries when building the ./configure test program. These changes almost certainly break *nix, so should be fixed before merge. --- configure.in | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index 0a01862..f111355 100644 --- a/configure.in +++ b/configure.in @@ -559,7 +559,7 @@ dnl There are no packages for Debian or Redhat as of this patch if test "$upnp" = "true"; then AC_DEFINE(MINIUPNPC, 1, [Define to 1 if we are building with UPnP.]) - TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc], + TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc -lws2_32 -liphlpapi], [#include <miniupnpc/miniwget.h> #include <miniupnpc/miniupnpc.h> #include <miniupnpc/upnpcommands.h>], @@ -567,7 +567,7 @@ if test "$upnp" = "true"; then const char * minissdpdsock, int sameport);], [upnpDiscover(1, 0, 0, 0); exit(0);], [--with-libminiupnpc-dir], - [/usr/lib/]) + [/local/lib/]) fi AC_SYS_LARGEFILE

1 0

[tor/master] XXX Use static linking on Windows because miniupnpc doesn't work as DLL
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 0ea28ccea0034ef846b0600f9b683d3b341299e6 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Jul 21 13:47:16 2011 +0100 XXX Use static linking on Windows because miniupnpc doesn't work as DLL --- src/tools/tor-fw-helper/tor-fw-helper-upnp.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c index 18ca563..55bae11 100644 --- a/src/tools/tor-fw-helper/tor-fw-helper-upnp.c +++ b/src/tools/tor-fw-helper/tor-fw-helper-upnp.c @@ -9,6 +9,7 @@ #include "orconfig.h" #ifdef MINIUPNPC +#define STATICLIB #include <stdint.h> #include <string.h> #include <stdio.h>

1 0

[tor/master] XXX Link in libiphlpapi (should be disabled on non Win32 platforms)
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 8db98c13b5cdc8eb11dfb593aea380334dc2ef7a Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Jul 21 13:47:58 2011 +0100 XXX Link in libiphlpapi (should be disabled on non Win32 platforms) --- src/tools/tor-fw-helper/Makefile.am | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am index 75878f2..05cea77 100644 --- a/src/tools/tor-fw-helper/Makefile.am +++ b/src/tools/tor-fw-helper/Makefile.am @@ -25,7 +25,7 @@ endif if MINIUPNPC miniupnpc_ldflags = @TOR_LDFLAGS_libminiupnpc@ -miniupnpc_ldadd = -lminiupnpc -lm +miniupnpc_ldadd = -lminiupnpc -lm -liphlpapi miniupnpc_cppflags = @TOR_CPPFLAGS_libminiupnpc@ else miniupnpc_ldflags =

1 0

[tor/master] Under Windows, call WSAStartup before using the network
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 2002d4acdfac823c03cca3ed92de7f60b3272d86 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Jul 21 14:14:57 2011 +0100 Under Windows, call WSAStartup before using the network --- src/tools/tor-fw-helper/tor-fw-helper.c | 33 ++++++++++++++++++++++++++++++- 1 files changed, 32 insertions(+), 1 deletions(-) diff --git a/src/tools/tor-fw-helper/tor-fw-helper.c b/src/tools/tor-fw-helper/tor-fw-helper.c index 20d60d7..926fa61 100644 --- a/src/tools/tor-fw-helper/tor-fw-helper.c +++ b/src/tools/tor-fw-helper/tor-fw-helper.c @@ -13,6 +13,7 @@ * later date. */ +#include "orconfig.h" #include <stdio.h> #include <stdint.h> #include <stdlib.h> @@ -20,7 +21,10 @@ #include <time.h> #include <string.h> -#include "orconfig.h" +#ifdef MS_WINDOWS +#include <winsock2.h> +#endif + #include "tor-fw-helper.h" #ifdef NAT_PMP #include "tor-fw-helper-natpmp.h" @@ -219,6 +223,29 @@ tor_fw_add_dir_port(tor_fw_options_t *tor_fw_options, } } +/** Called before we make any calls to network-related functions. + * (Some operating systems require their network libraries to be + * initialized.) (from common/compat.c) */ +static int +network_init(void) +{ +#ifdef MS_WINDOWS + /* This silly exercise is necessary before windows will allow + * gethostbyname to work. */ + WSADATA WSAData; + int r; + r = WSAStartup(0x101, &WSAData); + if (r) { + fprintf(stderr, "E: Error initializing Windows network layer - code was %d", r); + return -1; + } + /* WSAData.iMaxSockets might show the max sockets we're allowed to use. + * We might use it to complain if we're trying to be a server but have + * too few sockets available. */ +#endif + return 0; +} + int main(int argc, char **argv) { @@ -329,6 +356,10 @@ main(int argc, char **argv) tor_fw_options.public_dir_port); } + // Initialize networking + if (network_init()) + exit(1); + // Initalize the various fw-helper backend helpers r = init_backends(&tor_fw_options, &backend_state); if (r)

1 0

[tor/master] Generalize process spawning so its test compiles (but fails) in Windows
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 35c89be02b535e2951b695429bd9b255afb2a7b2 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Jul 21 16:34:48 2011 +0100 Generalize process spawning so its test compiles (but fails) in Windows - pid, stdout/stderr_pipe now encapsulated in process_handle - read_all replaced by tor_read_all_from_process_stdin/stderr - waitpid replaced by tor_get_exit_code Untested on *nix --- src/common/util.c | 78 ++++++++++++++++++++++++++++++++++++++++++-------- src/common/util.h | 23 +++++++++++++- src/test/test_util.c | 33 +++++++++------------ 3 files changed, 101 insertions(+), 33 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index b95ee3a..5bc7a80 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2978,14 +2978,15 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, * -1. Some parts of this code are based on the POSIX subprocess module from * Python. */ -int -tor_spawn_background(const char *const filename, int *stdout_read, - int *stderr_read, const char **argv) +process_handle_t +tor_spawn_background(const char *const filename, const char **argv) { + process_handle_t process_handle; #ifdef MS_WINDOWS - (void) filename; (void) stdout_read; (void) stderr_read; (void) argv; + (void) filename; (void) argv; log_warn(LD_BUG, "not yet implemented on Windows."); - return -1; + process_handle.status = -1; + return process_handle; #else pid_t pid; int stdout_pipe[2]; @@ -3016,7 +3017,8 @@ tor_spawn_background(const char *const filename, int *stdout_read, log_warn(LD_GENERAL, "Failed to set up pipe for stdout communication with child process: %s", strerror(errno)); - return -1; + process_handle.status = -1; + return process_handle; } retval = pipe(stderr_pipe); @@ -3024,7 +3026,8 @@ tor_spawn_background(const char *const filename, int *stdout_read, log_warn(LD_GENERAL, "Failed to set up pipe for stderr communication with child process: %s", strerror(errno)); - return -1; + process_handle.status = -1; + return process_handle; } child_state = CHILD_STATE_MAXFD; @@ -3109,7 +3112,8 @@ tor_spawn_background(const char *const filename, int *stdout_read, (void) nbytes; _exit(255); - return -1; /* Never reached, but avoids compiler warning */ + process_handle.status = -1; + return process_handle; /* Never reached, but avoids compiler warning */ } /* In parent */ @@ -3120,11 +3124,12 @@ tor_spawn_background(const char *const filename, int *stdout_read, close(stdout_pipe[1]); close(stderr_pipe[0]); close(stderr_pipe[1]); - return -1; + process_handle.status = -1; + return process_handle; } /* Return read end of the pipes to caller, and close write end */ - *stdout_read = stdout_pipe[0]; + process_handle.stdout_pipe = stdout_pipe[0]; retval = close(stdout_pipe[1]); if (-1 == retval) { @@ -3135,7 +3140,7 @@ tor_spawn_background(const char *const filename, int *stdout_read, needs to know about the pid in order to reap it later */ } - *stderr_read = stderr_pipe[0]; + process_handle.stderr_pipe = stderr_pipe[0]; retval = close(stderr_pipe[1]); if (-1 == retval) { @@ -3146,7 +3151,56 @@ tor_spawn_background(const char *const filename, int *stdout_read, needs to know about the pid in order to reap it later */ } - return pid; + process_handle.status = 0; + process_handle.pid = pid; + return process_handle; +#endif +} + +int +tor_get_exit_code(const process_handle_t process_handle) +{ +#ifdef MS_WINDOWS + log_warn(LD_BUG, "not yet implemented on Windows."); + return -1; +#else + int stat_loc; + + retval = waitpid(process_handle.pid, &stat_loc, 0); + if (retval != process_handle.pid) { + log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid, + sterror(errno)); + return -1; + } + + if (!WIFEXITED(stat_loc)) { + log_warn(LD_GENERAL, "Process %d did not exit normally", process_handle.pid); + return -1; + } + + return WEXITSTATUS(stat_loc); +#endif // MS_WINDOWS +} + +ssize_t +tor_read_all_from_process_stdin(const process_handle_t process_handle, + char *buf, size_t count) +{ +#ifdef MS_WINDOWS + return -1; +#else + return read_all(process_handle.stdin_pipe, buf, count, 0); +#endif +} + +ssize_t +tor_read_all_from_process_stderr(const process_handle_t process_handle, + char *buf, size_t count) +{ +#ifdef MS_WINDOWS + return -1; +#else + return read_all(process_handle.stderr_pipe, buf, count, 0); #endif } diff --git a/src/common/util.h b/src/common/util.h index 6496c42..2cf57a1 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -347,8 +347,27 @@ HANDLE load_windows_system_library(const TCHAR *library_name); #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */ -int tor_spawn_background(const char *const filename, int *stdout_read, - int *stderr_read, const char **argv); + +typedef struct process_handle_s { + int status; +#ifdef MS_WINDOWS + HANDLE stdout_pipe; + HANDLE stderr_pipe; + HANDLE pid; +#else + int stdout_pipe; + int stderr_pipe; + int pid; +#endif // MS_WINDOWS +} process_handle_t; + +process_handle_t tor_spawn_background(const char *const filename, + const char **argv); +int tor_get_exit_code(const process_handle_t pid); +ssize_t tor_read_all_from_process_stdin(const process_handle_t process_handle, + char *buf, size_t count); +ssize_t tor_read_all_from_process_stderr(const process_handle_t process_handle, + char *buf, size_t count); void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno); diff --git a/src/test/test_util.c b/src/test/test_util.c index c4769e6..fce53b8 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1376,42 +1376,38 @@ test_util_fgets_eagain(void *ptr) } #endif -#ifndef MS_WINDOWS /** 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) { - int stdout_pipe=-1, stderr_pipe=-1; - int retval, stat_loc; - pid_t pid; + int retval; ssize_t pos; + process_handle_t process_handle; char stdout_buf[100], stderr_buf[100]; /* Start the program */ - retval = tor_spawn_background(argv[0], &stdout_pipe, &stderr_pipe, argv); - tt_int_op(retval, >, 0); - tt_int_op(stdout_pipe, >, 0); - tt_int_op(stderr_pipe, >, 0); - pid = retval; + process_handle = tor_spawn_background(argv[0], argv); + + tt_int_op(process_handle.status, ==, 0); + tt_int_op(process_handle.stdout_pipe, >, 0); + tt_int_op(process_handle.stderr_pipe, >, 0); /* Check stdout */ - pos = read_all(stdout_pipe, stdout_buf, sizeof(stdout_buf) - 1, 0); + pos = tor_read_all_from_process_stdin(process_handle, stdout_buf, + sizeof(stdout_buf) - 1); tt_assert(pos >= 0); stdout_buf[pos] = '\0'; tt_int_op(pos, ==, strlen(expected_out)); tt_str_op(stdout_buf, ==, expected_out); /* Check it terminated correctly */ - retval = waitpid(pid, &stat_loc, 0); - tt_int_op(retval, ==, pid); - tt_assert(WIFEXITED(stat_loc)); - tt_int_op(WEXITSTATUS(stat_loc), ==, expected_exit); - tt_assert(!WIFSIGNALED(stat_loc)); - tt_assert(!WIFSTOPPED(stat_loc)); + retval = tor_get_exit_code(process_handle); + tt_int_op(retval, ==, expected_exit); /* Check stderr */ - pos = read_all(stderr_pipe, stderr_buf, sizeof(stderr_buf) - 1, 0); + pos = tor_read_all_from_process_stderr(process_handle, stderr_buf, + sizeof(stderr_buf) - 1); tt_assert(pos >= 0); stderr_buf[pos] = '\0'; tt_int_op(pos, ==, strlen(expected_err)); @@ -1447,7 +1443,6 @@ test_util_spawn_background_fail(void *ptr) run_util_spawn_background(argv, expected_out, expected_err, 255); } -#endif static void test_util_di_ops(void) @@ -1533,9 +1528,9 @@ struct testcase_t util_tests[] = { UTIL_TEST(exit_status, 0), #ifndef MS_WINDOWS UTIL_TEST(fgets_eagain, TT_SKIP), +#endif UTIL_TEST(spawn_background_ok, 0), UTIL_TEST(spawn_background_fail, 0), -#endif END_OF_TESTCASES };

1 0