commit f1ff65dfad800ed89e5b01c1c5b4b77c10a438b8 Author: Steven Murdoch Steven.Murdoch@cl.cam.ac.uk Date: Mon Aug 29 00:30:18 2011 +0100
Replace two magic tristates with #define'd names
- process_handle_t.status - return value of tor_get_exit_code() --- src/common/util.c | 64 ++++++++++++++++++++++++------------------------- src/common/util.h | 13 +++++++++- src/test/test_util.c | 18 ++++++++------ 3 files changed, 53 insertions(+), 42 deletions(-)
diff --git a/src/common/util.c b/src/common/util.c index 91b83db..371e09b 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3082,7 +3082,7 @@ tor_spawn_background(const char *const filename, const char **argv,
/* Assume failure to start process */ memset(process_handle, 0, sizeof(process_handle_t)); - process_handle->status = -1; + process_handle->status = PROCESS_STATUS_ERROR;
/* Set up pipe for stdout */ if (!CreatePipe(&stdout_pipe_read, &stdout_pipe_write, &saAttr, 0)) { @@ -3157,7 +3157,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 = 1; + process_handle->status = PROCESS_STATUS_RUNNING; }
/* TODO: Close pipes on exit */ @@ -3183,7 +3183,7 @@ tor_spawn_background(const char *const filename, const char **argv,
/* Assume failure to start */ memset(process_handle, 0, sizeof(process_handle_t)); - process_handle->status = -1; + process_handle->status = PROCESS_STATUS_ERROR;
/* 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 */ @@ -3329,7 +3329,7 @@ tor_spawn_background(const char *const filename, const char **argv, strerror(errno)); }
- process_handle->status = 1; + process_handle->status = PROCESS_STATUS_RUNNING; /* 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); @@ -3341,16 +3341,16 @@ tor_spawn_background(const char *const filename, const char **argv, #endif // MS_WINDOWS }
-/* Get the exit code of a process specified by <b>process_handle</b> and - * store it in <b>exit_code</b>, if set to a non-NULL value. If - * <b>block</b> is set to true, the call will block until the process has - * exited. Otherwise if the process is still running, the function will - * return -2, and exit_code will be left unchanged. Returns 0 if the - * process did exit. If there is a failure, -1 will be returned and the - * contents of exit_code (if non-NULL) will be undefined. N.B. Under *nix - * operating systems, this will probably not work in Tor, because - * waitpid() is called in main.c to reap any terminated child - * processes.*/ +/* Get the exit code of a process specified by <b>process_handle</b> and store + * it in <b>exit_code</b>, if set to a non-NULL value. If <b>block</b> is set + * to true, the call will block until the process has exited. Otherwise if + * the process is still running, the function will return + * PROCESS_EXIT_RUNNING, and exit_code will be left unchanged. Returns + * PROCESS_EXIT_EXITED if the process did exit. If there is a failure, + * PROCESS_EXIT_ERROR will be returned and the contents of exit_code (if + * non-NULL) will be undefined. N.B. Under *nix operating systems, this will + * probably not work in Tor, because waitpid() is called in main.c to reap any + * terminated child processes.*/ int tor_get_exit_code(const process_handle_t process_handle, int block, int *exit_code) @@ -3365,17 +3365,17 @@ tor_get_exit_code(const process_handle_t process_handle, if (retval != WAIT_OBJECT_0) { log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s", (int)retval, format_win32_error(GetLastError())); - return -1; + return PROCESS_EXIT_ERROR; } } else { retval = WaitForSingleObject(process_handle.pid.hProcess, 0); if (WAIT_TIMEOUT == retval) { /* Process has not exited */ - return -2; + return PROCESS_EXIT_RUNNING; } else if (retval != WAIT_OBJECT_0) { log_warn(LD_GENERAL, "WaitForSingleObject() failed (%d): %s", (int)retval, format_win32_error(GetLastError())); - return -1; + return PROCESS_EXIT_ERROR; } }
@@ -3385,7 +3385,7 @@ tor_get_exit_code(const process_handle_t process_handle, if (!success) { log_warn(LD_GENERAL, "GetExitCodeProcess() failed: %s", format_win32_error(GetLastError())); - return -1; + return PROCESS_EXIT_ERROR; } } #else @@ -3395,24 +3395,24 @@ tor_get_exit_code(const process_handle_t process_handle, retval = waitpid(process_handle.pid, &stat_loc, block?0:WNOHANG); if (!block && 0 == retval) { /* Process has not exited */ - return -2; + return PROCESS_EXIT_RUNNING; } else if (retval != process_handle.pid) { log_warn(LD_GENERAL, "waitpid() failed for PID %d: %s", process_handle.pid, strerror(errno)); - return -1; + return PROCESS_EXIT_ERROR; }
if (!WIFEXITED(stat_loc)) { log_warn(LD_GENERAL, "Process %d did not exit normally", process_handle.pid); - return -1; + return PROCESS_EXIT_ERROR; }
if (exit_code != NULL) *exit_code = WEXITSTATUS(stat_loc); #endif // MS_WINDOWS
- return 0; + return PROCESS_EXIT_EXITED; }
#ifdef MS_WINDOWS @@ -3648,11 +3648,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port,
/* Static variables are initialized to zero, so child_handle.status=0 * which corresponds to it not running on startup */ -#ifdef MS_WINDOWS static process_handle_t child_handle; -#else - static process_handle_t child_handle; -#endif
static time_t time_to_run_helper = 0; int stdout_status, stderr_status, retval; @@ -3678,7 +3674,8 @@ 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 (child_handle.status <= 0 && time_to_run_helper < now) { + if (child_handle.status != PROCESS_STATUS_RUNNING && + time_to_run_helper < now) { /* Assume tor-fw-helper will succeed, start it later*/ time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_SUCCESS;
@@ -3688,7 +3685,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, #else tor_spawn_background(filename, argv, &child_handle); #endif - if (child_handle.status < 0) { + if (PROCESS_STATUS_ERROR == child_handle.status) { log_warn(LD_GENERAL, "Failed to start port forwarding helper %s", filename); time_to_run_helper = now + TIME_TO_EXEC_FWHELPER_FAIL; @@ -3705,7 +3702,7 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, }
/* If child is running, read from its stdout and stderr) */ - if (child_handle.status > 0) { + if (PROCESS_STATUS_RUNNING == child_handle.status) { /* Read from stdout/stderr and log result */ retval = 0; #ifdef MS_WINDOWS @@ -3729,8 +3726,9 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, /* There was a failure */ retval = -1; #ifdef MS_WINDOWS - else if (tor_get_exit_code(child_handle, 0, NULL) >= 0) { - /* process has exited */ + else if (tor_get_exit_code(child_handle, 0, NULL) != + PROCESS_EXIT_RUNNING) { + /* process has exited or there was an error */ /* TODO: Do something with the process return value */ /* TODO: What if the process output something since * between log_from_handle and tor_get_exit_code? */ @@ -3751,10 +3749,10 @@ tor_check_port_forwarding(const char *filename, int dir_port, int or_port, if (0 != retval) { if (1 == retval) { log_info(LD_GENERAL, "Port forwarding helper terminated"); - child_handle.status = 0; + child_handle.status = PROCESS_STATUS_NOTRUNNING; } else { log_warn(LD_GENERAL, "Failed to read from port forwarding helper"); - child_handle.status = -1; + child_handle.status = PROCESS_STATUS_ERROR; }
/* TODO: The child might not actually be finished (maybe it failed or diff --git a/src/common/util.h b/src/common/util.h index e025265..d8c7370 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -355,8 +355,14 @@ HANDLE load_windows_system_library(const TCHAR *library_name); #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */
+/* Values of process_handle_t.status. PROCESS_STATUS_NOTRUNNING must be + * 0 because tor_check_port_forwarding depends on this being the initial + * statue of the static instance of process_handle_t */ +#define PROCESS_STATUS_NOTRUNNING 0 +#define PROCESS_STATUS_RUNNING 1 +#define PROCESS_STATUS_ERROR -1 typedef struct process_handle_s { - int status; // 0: not running; 1: running; -1: error + int status; #ifdef MS_WINDOWS HANDLE stdout_pipe; HANDLE stderr_pipe; @@ -372,6 +378,11 @@ typedef struct process_handle_s {
int tor_spawn_background(const char *const filename, const char **argv, process_handle_t *process_handle); + +/* Return values of tor_get_exit_code() */ +#define PROCESS_EXIT_RUNNING 1 +#define PROCESS_EXIT_EXITED 0 +#define PROCESS_EXIT_ERROR -1 int tor_get_exit_code(const process_handle_t process_handle, int block, int *exit_code); #ifdef MS_WINDOWS diff --git a/src/test/test_util.c b/src/test/test_util.c index 40ab098..4568fde 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1397,7 +1397,7 @@ run_util_spawn_background(const char *argv[], const char *expected_out, tt_int_op(process_handle.status, ==, expected_status);
/* If the process failed to start, don't bother continuing */ - if (process_handle.status == -1) + if (process_handle.status == PROCESS_STATUS_ERROR) return;
tt_int_op(process_handle.stdout_pipe, >, 0); @@ -1413,7 +1413,7 @@ run_util_spawn_background(const char *argv[], const char *expected_out,
/* Check it terminated correctly */ retval = tor_get_exit_code(process_handle, 1, &exit_code); - tt_int_op(retval, ==, 0); + tt_int_op(retval, ==, PROCESS_EXIT_EXITED); tt_int_op(exit_code, ==, expected_exit); // TODO: Make test-child exit with something other than 0
@@ -1445,7 +1445,8 @@ test_util_spawn_background_ok(void *ptr)
(void)ptr;
- run_util_spawn_background(argv, expected_out, expected_err, 0, 1); + run_util_spawn_background(argv, expected_out, expected_err, 0, + PROCESS_STATUS_RUNNING); }
/** Check that failing to find the executable works as expected */ @@ -1457,14 +1458,15 @@ test_util_spawn_background_fail(void *ptr) const char *expected_out = "ERR: Failed to spawn background process " "- code 9/2\n"; const char *expected_err = ""; - const int expected_status = -1; + const int expected_status = PROCESS_STATUS_ERROR; #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 = 1; + /* TODO: Once we can signal failure to exec, set this to be + * PROCESS_STATUS_ERROR */ + const int expected_status = PROCESS_STATUS_RUNNING; #endif
(void)ptr; @@ -1479,7 +1481,7 @@ static void test_util_spawn_background_partial_read(void *ptr) { const int expected_exit = 0; - const int expected_status = 1; + const int expected_status = PROCESS_STATUS_RUNNING;
int retval, exit_code; ssize_t pos; @@ -1536,7 +1538,7 @@ test_util_spawn_background_partial_read(void *ptr)
/* Check it terminated correctly */ retval = tor_get_exit_code(process_handle, 1, &exit_code); - tt_int_op(retval, ==, 0); + tt_int_op(retval, ==, PROCESS_EXIT_EXITED); tt_int_op(exit_code, ==, expected_exit); // TODO: Make test-child exit with something other than 0