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

[tor/master] Rename tor_join_cmdline to tor_join_win_cmdline; tweak doxygen
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 2778cdd6711b4c1268b6a8313d9bd8d9fb62e6aa Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Aug 30 16:00:08 2011 -0400 Rename tor_join_cmdline to tor_join_win_cmdline; tweak doxygen --- src/common/util.c | 31 +++++++++++++++---------------- src/common/util.h | 2 +- src/test/test_util.c | 14 +++++++------- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 6093662..2fa79b8 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2957,10 +2957,10 @@ load_windows_system_library(const TCHAR *library_name) } #endif -/* Format a single argument for being put on a Windows command line. +/** Format a single argument for being put on a Windows command line. * Returns a newly allocated string */ static char * -format_cmdline_argument(const char *arg) +format_win_cmdline_argument(const char *arg) { char *formatted_arg; char need_quotes; @@ -3026,12 +3026,12 @@ format_cmdline_argument(const char *arg) return formatted_arg; } -/* Format a command line for use on Windows, which takes the command as a +/** Format a command line for use on Windows, which takes the command as a * string rather than string array. Follows the rules from "Parsing C++ * Command-Line Arguments" in MSDN. Algorithm based on list2cmdline in the * Python subprocess module. Returns a newly allocated string */ char * -tor_join_cmdline(const char *argv[]) +tor_join_win_cmdline(const char *argv[]) { smartlist_t *argv_list; char *joined_argv; @@ -3040,7 +3040,7 @@ tor_join_cmdline(const char *argv[]) /* Format each argument and put the result in a smartlist */ argv_list = smartlist_create(); for (i=0; argv[i] != NULL; i++) { - smartlist_add(argv_list, (void *)format_cmdline_argument(argv[i])); + smartlist_add(argv_list, (void *)format_win_cmdline_argument(argv[i])); } /* Join the arguments with whitespace */ @@ -3217,7 +3217,7 @@ tor_spawn_background(const char *const filename, const char **argv, /* Windows expects argv to be a whitespace delimited string, so join argv up */ - joined_argv = tor_join_cmdline(argv); + joined_argv = tor_join_win_cmdline(argv); ZeroMemory(&(process_handle->pid), sizeof(PROCESS_INFORMATION)); ZeroMemory(&siStartInfo, sizeof(STARTUPINFO)); @@ -3437,7 +3437,7 @@ tor_spawn_background(const char *const filename, const char **argv, #endif // MS_WINDOWS } -/* Get the exit code of a process specified by process_handle and store +/** Get the exit code of a process specified by process_handle and store * it in exit_code, if set to a non-NULL value. If block is set * to true, the call will block until the process has exited. Otherwise if * the process is still running, the function will return @@ -3575,7 +3575,7 @@ tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) } #endif -/* Read from stdout of a process until the process exits. */ +/** Read from stdout of a process until the process exits. */ ssize_t tor_read_all_from_process_stdout(const process_handle_t process_handle, char *buf, size_t count) @@ -3588,7 +3588,7 @@ tor_read_all_from_process_stdout(const process_handle_t process_handle, #endif } -/* Read from stdout of a process until the process exits. */ +/** Read from stdout of a process until the process exits. */ ssize_t tor_read_all_from_process_stderr(const process_handle_t process_handle, char *buf, size_t count) @@ -3601,12 +3601,11 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, #endif } -/* Split buf into lines, and add to smartlist. The buffer buf will be +/** Split buf into lines, and add to smartlist. The buffer buf will be * modified. The resulting smartlist will consist of pointers to buf, so there - * is no need to free the contents of sl. buf must be a NULL terminated + * is no need to free the contents of sl. buf must be a NUL-terminated * string. len should be set to the length of the buffer excluding the - * NULL. Non-printable characters (including NULL) will be replaced with "." */ - + * NUL. Non-printable characters (including NUL) will be replaced with "." */ int tor_split_lines(smartlist_t *sl, char *buf, int len) { @@ -3627,7 +3626,7 @@ tor_split_lines(smartlist_t *sl, char *buf, int len) buf[cur] = '\0'; /* Point cur to the next line */ cur++; - /* Line starts at start and ends with a null */ + /* Line starts at start and ends with a nul */ break; } else { if (!TOR_ISPRINT(buf[cur])) @@ -3646,8 +3645,8 @@ tor_split_lines(smartlist_t *sl, char *buf, int len) } } /* We are at the end of the line or end of string. If in_line is true there - * is a line which starts at buf+start and ends at a NULL. cur points to - * the character after the NULL. */ + * is a line which starts at buf+start and ends at a NUL. cur points to + * the character after the NUL. */ if (in_line) smartlist_add(sl, (void *)(buf+start)); in_line = 0; diff --git a/src/common/util.h b/src/common/util.h index 853ac8d..3029425 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -394,7 +394,7 @@ 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, char *buf, size_t count); -char *tor_join_cmdline(const char *argv[]); +char *tor_join_win_cmdline(const char *argv[]); 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 a3bc003..3b5d85f 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1558,12 +1558,12 @@ test_util_spawn_background_partial_read(void *ptr) * Test that we can properly format q Windows command line */ static void -test_util_join_cmdline(void *ptr) +test_util_join_win_cmdline(void *ptr) { - /* Based on some test cases from "Parsing C++ Command-Line Arguments" in MSDN - * but we don't exercise all quoting rules because tor_join_cmdline will try - * to only generate simple cases for the child process to parse; i.e. we - * never embed quoted strings in arguments. */ + /* Based on some test cases from "Parsing C++ Command-Line Arguments" in + * MSDN but we don't exercise all quoting rules because tor_join_win_cmdline + * will try to only generate simple cases for the child process to parse; + * i.e. we never embed quoted strings in arguments. */ const char *argvs[][4] = { {"a", "bb", "CCC", NULL}, // Normal @@ -1596,7 +1596,7 @@ test_util_join_cmdline(void *ptr) for (i=0; cmdlines[i]!=NULL; i++) { log_info(LD_GENERAL, "Joining argvs[%d], expecting <%s>", i, cmdlines[i]); - joined_argv = tor_join_cmdline(argvs[i]); + joined_argv = tor_join_win_cmdline(argvs[i]); tt_str_op(joined_argv, ==, cmdlines[i]); tor_free(joined_argv); } @@ -1754,7 +1754,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(spawn_background_ok, 0), UTIL_TEST(spawn_background_fail, 0), UTIL_TEST(spawn_background_partial_read, 0), - UTIL_TEST(join_cmdline, 0), + UTIL_TEST(join_win_cmdline, 0), UTIL_TEST(split_lines, 0), END_OF_TESTCASES };

1 0

[tor/master] Merge remote branch 'origin/master' into bug2046
by nickm＠torproject.org 30 Aug '11

30 Aug '11

1 0

[tor/master] Link and build tor-fw-helper on Windows
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 2ad336f999781db211d92332e657398829c8799c Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Fri Aug 19 14:47:44 2011 +0100 Link and build tor-fw-helper on Windows - Update configure script to test for libminiupnpc along with the libws2_32 and libiphlpapi libraries required by libminiupnpc - When building tor-fw-helper, link in libiphlpapi - Link in libminiupnpc statically becasue I could not get the DLL to link properly - Call WSAStartup before doing network operations - Fix up a compiler warning about uninitialized backend_state N.B. The changes to configure.in and Makefile.am will break on non- Windows platforms. --- configure.in | 2 +- src/tools/tor-fw-helper/Makefile.am | 2 +- src/tools/tor-fw-helper/tor-fw-helper-upnp.c | 3 ++ src/tools/tor-fw-helper/tor-fw-helper.c | 34 +++++++++++++++++++++++++- 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/configure.in b/configure.in index 4a89df6..1c45f18 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>], diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am index 77ff63f..39aabc7 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 = 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..c4b14a8 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,9 @@ #include "orconfig.h" #ifdef MINIUPNPC +#ifdef MS_WINDOWS +#define STATICLIB +#endif #include <stdint.h> #include <string.h> #include <stdio.h> diff --git a/src/tools/tor-fw-helper/tor-fw-helper.c b/src/tools/tor-fw-helper/tor-fw-helper.c index 20d60d7..cb8e0cd 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) { @@ -229,6 +256,7 @@ main(int argc, char **argv) backends_t backend_state; memset(&tor_fw_options, 0, sizeof(tor_fw_options)); + memset(&backend_state, 0, sizeof(backend_state)); while (1) { int option_index = 0; @@ -329,6 +357,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] Only link ws2_32 and iphlapi on windows.
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 269c0b4633a5b41b2d37cf9c877fe37c4ea4a1f9 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Aug 19 13:20:15 2011 -0400 Only link ws2_32 and iphlapi on windows. This is a tweak for the tor-fw-helper port to windows. --- configure.in | 4 +++- src/tools/tor-fw-helper/Makefile.am | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/configure.in b/configure.in index 1c45f18..f9e98f0 100644 --- a/configure.in +++ b/configure.in @@ -325,6 +325,7 @@ dnl Where do you live, libevent? And how do we call you? if test "$bwin32" = true; then TOR_LIB_WS32=-lws2_32 + TOR_LIB_IPHLAPI=-liphlapi # Some of the cargo-cults recommend -lwsock32 as well, but I don't # think it's actually necessary. TOR_LIB_GDI=-lgdi32 @@ -334,6 +335,7 @@ else fi AC_SUBST(TOR_LIB_WS32) AC_SUBST(TOR_LIB_GDI) +AC_SUBST(TOR_LIB_IPHLAPI) dnl We need to do this before we try our disgusting hack below. AC_CHECK_HEADERS([sys/types.h]) @@ -559,7 +561,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 -lws2_32 -liphlpapi], + TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc $TOR_LIB_WS32 $TOR_LIB_IPHLAPI], [#include <miniupnpc/miniwget.h> #include <miniupnpc/miniupnpc.h> #include <miniupnpc/upnpcommands.h>], diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am index 39aabc7..127a000 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 -liphlpapi +miniupnpc_ldadd = -lminiupnpc -lm @TOR_LIB_IPHLAPI@ miniupnpc_cppflags = @TOR_CPPFLAGS_libminiupnpc@ else miniupnpc_ldflags =

1 0

[tor/master] Appease "make check spaces"
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit c5e74fc60db2b0c763d55f5bca8a32dc4bc05fe5 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Mon Aug 22 16:31:30 2011 +0100 Appease "make check spaces" --- src/tools/tor-fw-helper/tor-fw-helper.c | 3 ++- 1 files changed, 2 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 cb8e0cd..0022397 100644 --- a/src/tools/tor-fw-helper/tor-fw-helper.c +++ b/src/tools/tor-fw-helper/tor-fw-helper.c @@ -236,7 +236,8 @@ network_init(void) int r; r = WSAStartup(0x101, &WSAData); if (r) { - fprintf(stderr, "E: Error initializing Windows network layer - code was %d", 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.

1 0

[tor/master] Merge branch 'bug1983-port-tor-fw-helper-to-windows' into bug2046
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 6443a756df2a94e70bc221830d49efb024e0a2ce Merge: cc5b6d6 850d8c9 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Mon Aug 22 17:53:17 2011 +0100 Merge branch 'bug1983-port-tor-fw-helper-to-windows' into bug2046 Conflicts: configure.in src/tools/tor-fw-helper/Makefile.am src/tools/tor-fw-helper/tor-fw-helper-upnp.c src/tools/tor-fw-helper/tor-fw-helper.c changes/bug3752 | 5 +++++ configure.in | 4 +++- src/common/compat_libevent.c | 11 +++++++++++ src/common/compat_libevent.h | 1 + src/common/tortls.c | 2 +- src/or/buffers.c | 14 +++++++------- src/or/connection.c | 1 + src/tools/tor-fw-helper/Makefile.am | 2 +- src/tools/tor-fw-helper/tor-fw-helper-upnp.c | 2 ++ src/tools/tor-fw-helper/tor-fw-helper.c | 4 +++- 10 files changed, 35 insertions(+), 11 deletions(-)

1 0

[tor/master] Correct reference to libiphlpapi from libiphlapi
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 850d8c9eb8a847d3d35d4e39acf2d84afe8dffa4 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Mon Aug 22 17:38:22 2011 +0100 Correct reference to libiphlpapi from libiphlapi --- configure.in | 6 +++--- src/tools/tor-fw-helper/Makefile.am | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/configure.in b/configure.in index f9e98f0..26792a6 100644 --- a/configure.in +++ b/configure.in @@ -325,7 +325,7 @@ dnl Where do you live, libevent? And how do we call you? if test "$bwin32" = true; then TOR_LIB_WS32=-lws2_32 - TOR_LIB_IPHLAPI=-liphlapi + TOR_LIB_IPHLPAPI=-liphlpapi # Some of the cargo-cults recommend -lwsock32 as well, but I don't # think it's actually necessary. TOR_LIB_GDI=-lgdi32 @@ -335,7 +335,7 @@ else fi AC_SUBST(TOR_LIB_WS32) AC_SUBST(TOR_LIB_GDI) -AC_SUBST(TOR_LIB_IPHLAPI) +AC_SUBST(TOR_LIB_IPHLPAPI) dnl We need to do this before we try our disgusting hack below. AC_CHECK_HEADERS([sys/types.h]) @@ -561,7 +561,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_LIB_WS32 $TOR_LIB_IPHLAPI], + TOR_SEARCH_LIBRARY(libminiupnpc, $trylibminiupnpcdir, [-lminiupnpc $TOR_LIB_WS32 $TOR_LIB_IPHLPAPI], [#include <miniupnpc/miniwget.h> #include <miniupnpc/miniupnpc.h> #include <miniupnpc/upnpcommands.h>], diff --git a/src/tools/tor-fw-helper/Makefile.am b/src/tools/tor-fw-helper/Makefile.am index 127a000..8f64ad2 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 @TOR_LIB_IPHLAPI@ +miniupnpc_ldadd = -lminiupnpc -lm @TOR_LIB_IPHLPAPI@ miniupnpc_cppflags = @TOR_CPPFLAGS_libminiupnpc@ else miniupnpc_ldflags =

1 0

[tor/master] Revert change to libminiupnpc search path
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit a78ffd9e45a270b9979dcdc0d376f33bd95d7f09 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Mon Aug 22 17:55:40 2011 +0100 Revert change to libminiupnpc search path This was supposed to fix an oddity with library searching in autoconf but it didn't actually fix anything. --- configure.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/configure.in b/configure.in index e59f31b..26792a6 100644 --- a/configure.in +++ b/configure.in @@ -569,7 +569,7 @@ if test "$upnp" = "true"; then const char * minissdpdsock, int sameport);], [upnpDiscover(1, 0, 0, 0); exit(0);], [--with-libminiupnpc-dir], - [/local/lib/]) + [/usr/lib/]) fi AC_SYS_LARGEFILE

1 0

[tor/master] Add Windows version of tor_spawn_background and ancillary functions
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit fec902dd6024dd170d151409387df5319b6e2ad0 Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Thu Jul 21 19:26:19 2011 +0100 Add Windows version of tor_spawn_background and ancillary functions --- src/common/util.c | 141 ++++++++++++++++++++++++++++++++++++++++++++++---- src/common/util.h | 4 +- src/test/test_util.c | 14 ++++- 3 files changed, 143 insertions(+), 16 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 5bc7a80..3d39f59 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -2976,18 +2976,107 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, * and stderr, respectively, output of the child program can be read, and the * stdin of the child process shall be set to /dev/null. Otherwise returns * -1. Some parts of this code are based on the POSIX subprocess module from - * Python. + * Python, and example code from + * http://msdn.microsoft.com/en-us/library/ms682499%28v=vs.85%29.aspx. */ process_handle_t tor_spawn_background(const char *const filename, const char **argv) { process_handle_t process_handle; #ifdef MS_WINDOWS - (void) filename; (void) argv; - log_warn(LD_BUG, "not yet implemented on Windows."); + HANDLE stdout_pipe_read = NULL; + HANDLE stdout_pipe_write = NULL; + HANDLE stderr_pipe_read = NULL; + HANDLE stderr_pipe_write = NULL; + + SECURITY_ATTRIBUTES saAttr; + smartlist_t *argv_list; + char *joined_argv; + int i; + + saAttr.nLength = sizeof(SECURITY_ATTRIBUTES); + saAttr.bInheritHandle = TRUE; + saAttr.lpSecurityDescriptor = NULL; + process_handle.status = -1; + + /* Set up pipe for stdout */ + if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) { + log_warn(LD_GENERAL, + "Failed to create pipe for stdout communication with child process: %s", + format_win32_error(GetLastError())); + return process_handle; + } + if (!SetHandleInformation(stdout_pipe_read, HANDLE_FLAG_INHERIT, 0)) { + log_warn(LD_GENERAL, + "Failed to configure pipe for stdout communication with child process: %s", + format_win32_error(GetLastError())); + } + + /* Set up pipe for stderr */ + if (!CreatePipe(&stderr_pipe_read, &stderr_pipe_write, &saAttr, 0)) { + log_warn(LD_GENERAL, + "Failed to create pipe for stderr communication with child process: %s", + format_win32_error(GetLastError())); + return process_handle; + } + if (!SetHandleInformation(stderr_pipe_read, HANDLE_FLAG_INHERIT, 0)) { + log_warn(LD_GENERAL, + "Failed to configure pipe for stderr communication with child process: %s", + format_win32_error(GetLastError())); + } + + /* Create the child process */ + + /* Windows expects argv to be a whitespace delimited string, so join argv up */ + argv_list = smartlist_create(); + for (i=0; argv[i] != NULL; i++) { + smartlist_add(argv_list, (void *)argv[i]); + } + + joined_argv = smartlist_join_strings(argv_list, " ", 0, NULL); + + STARTUPINFO siStartInfo; + BOOL retval = FALSE; + + ZeroMemory(&process_handle.pid, sizeof(PROCESS_INFORMATION)); + ZeroMemory(&siStartInfo, sizeof(STARTUPINFO)); + siStartInfo.cb = sizeof(STARTUPINFO); + siStartInfo.hStdError = stderr_pipe_write; + siStartInfo.hStdOutput = stdout_pipe_write; + siStartInfo.hStdInput = NULL; + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; + + /* Create the child process */ + + retval = CreateProcess(filename, // module name + joined_argv, // command line + NULL, // process security attributes + NULL, // primary thread security attributes + TRUE, // handles are inherited + 0, // creation flags (TODO: set CREATE_NEW CONSOLE/PROCESS_GROUP to make GetExitCodeProcess() work?) + NULL, // use parent's environment + NULL, // use parent's current directory + &siStartInfo, // STARTUPINFO pointer + &process_handle.pid); // receives PROCESS_INFORMATION + + tor_free(joined_argv); + + if (!retval) { + log_warn(LD_GENERAL, + "Failed to create child process %s: %s", filename, + format_win32_error(GetLastError())); + } else { + // 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; + } + + // TODO: Close pipes on exit + return process_handle; -#else +#else // MS_WINDOWS pid_t pid; int stdout_pipe[2]; int stderr_pipe[2]; @@ -3154,15 +3243,25 @@ tor_spawn_background(const char *const filename, const char **argv) process_handle.status = 0; process_handle.pid = pid; return process_handle; -#endif +#endif // MS_WINDOWS } 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; + DWORD exit_code; + BOOL retval; + WaitForSingleObject(process_handle.pid.hProcess, INFINITE); + retval = GetExitCodeProcess(process_handle.pid.hProcess, &exit_code); + + if (!retval) { + log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s", + format_win32_error(GetLastError())); + return -1; + } else { + return exit_code; + } #else int stat_loc; @@ -3183,13 +3282,23 @@ tor_get_exit_code(const process_handle_t process_handle) } ssize_t -tor_read_all_from_process_stdin(const process_handle_t process_handle, +tor_read_all_from_process_stdout(const process_handle_t process_handle, char *buf, size_t count) { #ifdef MS_WINDOWS - return -1; + 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())); + return -1; + } else { + return bytes_read; + } #else - return read_all(process_handle.stdin_pipe, buf, count, 0); + return read_all(process_handle.stdout_pipe, buf, count, 0); #endif } @@ -3198,7 +3307,17 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, char *buf, size_t count) { #ifdef MS_WINDOWS - return -1; + 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; + } #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 2cf57a1..c111ba7 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -353,7 +353,7 @@ typedef struct process_handle_s { #ifdef MS_WINDOWS HANDLE stdout_pipe; HANDLE stderr_pipe; - HANDLE pid; + PROCESS_INFORMATION pid; #else int stdout_pipe; int stderr_pipe; @@ -364,7 +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_from_process_stdin(const process_handle_t process_handle, +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, char *buf, size_t count); diff --git a/src/test/test_util.c b/src/test/test_util.c index fce53b8..28030b7 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1394,24 +1394,25 @@ run_util_spawn_background(const char *argv[], const char *expected_out, tt_int_op(process_handle.stderr_pipe, >, 0); /* Check stdout */ - pos = tor_read_all_from_process_stdin(process_handle, stdout_buf, + pos = tor_read_all_from_process_stdout(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); + tt_int_op(pos, ==, strlen(expected_out)); /* 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_int_op(pos, ==, strlen(expected_err)); tt_str_op(stderr_buf, ==, expected_err); + tt_int_op(pos, ==, strlen(expected_err)); done: ; @@ -1421,9 +1422,16 @@ run_util_spawn_background(const char *argv[], const char *expected_out, static void test_util_spawn_background_ok(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\nDONE\r\n"; + const char *expected_err = "ERR\r\n"; +#else const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; const char *expected_out = "OUT\n--test\nDONE\n"; const char *expected_err = "ERR\n"; +#endif (void)ptr;

1 0

[tor/master] Use PeekNamedPipe to avoid blocking ReadFile when there is nothing to read
by nickm＠torproject.org 30 Aug '11

30 Aug '11

commit 2d5059e08e16ef704c3021e3ccdc2923be02c7cb Author: Steven Murdoch <Steven.Murdoch(a)cl.cam.ac.uk> Date: Fri Jul 22 21:12:00 2011 +0100 Use PeekNamedPipe to avoid blocking ReadFile when there is nothing to read --- src/common/util.c | 48 +++++++++++++++++++++++++++++++++++++++--------- src/test/test_util.c | 4 ++-- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index e9b7909..31d5586 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3287,27 +3287,55 @@ 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) +read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess) { size_t numread = 0; BOOL retval; - DWORD bytes_read; + DWORD byte_count; + BOOL process_exited = FALSE; if (count > SIZE_T_CEILING || count > SSIZE_T_MAX) return -1; while (numread != count) { - retval = ReadFile(h, buf+numread, count-numread, &bytes_read, NULL); + retval = PeekNamedPipe(h, NULL, 0, NULL, &byte_count, NULL); if (!retval) { log_warn(LD_GENERAL, - "Failed to read from stdin pipe: %s", + "Failed to peek from handle: %s", format_win32_error(GetLastError())); return -1; - } else if (0 == bytes_read) { + } 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 */ + if (NULL == hProcess) + continue; + + /* The process exited and there's nothing left to read from it */ + if (process_exited) + break; + + /* If process is not running, check for output one more time in case + it wrote something after the peek was performed. Otherwise keep on + waiting for output */ + byte_count = WaitForSingleObject(hProcess, 0); + if (WAIT_TIMEOUT != byte_count) + process_exited = TRUE; + + continue; + } + + retval = ReadFile(h, buf+numread, count-numread, &byte_count, NULL); + if (!retval) { + log_warn(LD_GENERAL, + "Failed to read from handle: %s", + format_win32_error(GetLastError())); + return -1; + } else if (0 == byte_count) { /* End of file */ - return bytes_read; + break; } - numread += bytes_read; + numread += byte_count; } return (ssize_t)numread; } @@ -3318,7 +3346,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); + return 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 @@ -3329,7 +3358,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); + return 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/test/test_util.c b/src/test/test_util.c index 63c8ad8..c6dc98d 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1431,11 +1431,11 @@ test_util_spawn_background_ok(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\nDONE\r\n"; + const char *expected_out = "OUT\r\n--test\r\nSLEEPING\r\nDONE\r\n"; const char *expected_err = "ERR\r\n"; #else const char *argv[] = {BUILDDIR "/src/test/test-child", "--test", NULL}; - const char *expected_out = "OUT\n--test\nDONE\n"; + const char *expected_out = "OUT\n--test\nSLEEPING\nDONE\n"; const char *expected_err = "ERR\n"; #endif

1 0