commit da34360952c0fbbd8effc2789ed72b86c8045531 Author: Steven Murdoch Steven.Murdoch@cl.cam.ac.uk Date: Tue Aug 30 14:55:51 2011 +0100

Factor out and re-write code for splitting lines from a handle

Now handles non-printable characters and will not output a spurious new-line if given a partial line. --- src/common/util.c | 89 +++++++++++++++++++++++++++++++++++++------------- src/common/util.h | 1 + src/test/test_util.c | 65 ++++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 23 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c index 87c6fb5..aa344bc 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3597,6 +3597,60 @@ tor_read_all_from_process_stderr(const process_handle_t process_handle, #endif }

+/* Split buf into lines, and add to smartlist. The buffer <b>buf</b> will be + * modified. The resulting smartlist will consist of pointers to buf, so there + * is no need to free the contents of sl. <b>buf</b> must be a NULL terminated + * string. <b>len</b> should be set to the length of the buffer excluding the + * NULL. Non-printable characters (including NULL) will be replaced with "." */ + +int +tor_split_lines(smartlist_t *sl, char *buf, int len) +{ + /* Index in buf of the start of the current line */ + int start = 0; + /* Index in buf of the current character being processed */ + int cur = 0; + /* Are we currently in a line */ + char in_line = 0; + + /* Loop over string */ + while (cur < len) { + /* Loop until end of line or end of string */ + for (; cur < len; cur++) { + if (in_line) { + if ('\r' == buf[cur] || '\n' == buf[cur]) { + /* End of line */ + buf[cur] = '\0'; + /* Point cur to the next line */ + cur++; + /* Line starts at start and ends with a null */ + break; + } else { + if (!TOR_ISPRINT(buf[cur])) + buf[cur] = '.'; + } + } else { + if ('\r' == buf[cur] || '\n' == buf[cur]) { + /* Skip leading vertical space */ + ; + } else { + in_line = 1; + start = cur; + if (!TOR_ISPRINT(buf[cur])) + buf[cur] = '.'; + } + } + } + /* 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. */ + if (in_line) + smartlist_add(sl, (void *)(buf+start)); + in_line = 0; + } + return smartlist_len(sl); +} + #ifdef MS_WINDOWS /** Read from stream, and send lines to log at the specified log level. * Returns -1 if there is a error reading, and 0 otherwise. @@ -3606,7 +3660,7 @@ log_from_handle(HANDLE *pipe, int severity) { char buf[256]; int pos; - int start, cur, next; + smartlist_t *lines;

pos = tor_read_all_handle(pipe, buf, sizeof(buf) - 1, NULL); if (pos < 0) { @@ -3626,28 +3680,17 @@ log_from_handle(HANDLE *pipe, int severity) buf[pos] = '\0'; log_debug(LD_GENERAL, "Subprocess had %d bytes to say", pos);

- /* Split buf into lines and log each one */ - 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++) { - /* On Windows \r means end of line */ - if ('\r' == buf[cur]) { - buf[cur] = '\0'; - next = cur + 1; - /* If \n follows, remove it too */ - if ((cur + 1) < pos && '\n' == buf[cur+1]) { - 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); - } + /* Split up the buffer */ + lines = smartlist_create(); + tor_split_lines(lines, buf, pos); + + /* Log each line */ + SMARTLIST_FOREACH(lines, char *, line, + { + log_fn(severity, LD_GENERAL, "Port forwarding helper says: %s", line); + }); + smartlist_free(lines); + return 0; }

diff --git a/src/common/util.h b/src/common/util.h index 9cdd8cb..853ac8d 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -385,6 +385,7 @@ int tor_spawn_background(const char *const filename, const char **argv, #define PROCESS_EXIT_ERROR -1 int tor_get_exit_code(const process_handle_t process_handle, int block, int *exit_code); +int tor_split_lines(struct smartlist_t *sl, char *buf, int len); #ifdef MS_WINDOWS ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count, HANDLE hProcess); diff --git a/src/test/test_util.c b/src/test/test_util.c index 9df7bc6..a3bc003 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1554,6 +1554,9 @@ 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) { @@ -1602,6 +1605,67 @@ test_util_join_cmdline(void *ptr) ; }

+#define MAX_SPLIT_LINE_COUNT 3 +struct split_lines_test_t { + const char *orig_line; // Line to be split (may contain \0's) + int orig_length; // Length of orig_line + const char *split_line[MAX_SPLIT_LINE_COUNT]; // Split lines +}; + +/** + * Test that we properly split a buffer into lines + */ +static void +test_util_split_lines(void *ptr) +{ + /* Test cases. orig_line of last test case must be NULL. + * The last element of split_line[i] must be NULL. */ + struct split_lines_test_t tests[] = { + {"", 0, {NULL}}, + {"foo", 3, {"foo", NULL}}, + {"\n\rfoo\n\rbar\r\n", 12, {"foo", "bar", NULL}}, + {"fo o\r\nb\tar", 10, {"fo o", "b.ar", NULL}}, + {"\x0f""f\0o\0\n\x01""b\0r\0\r", 12, {".f.o.", ".b.r.", NULL}}, + {NULL, 0, {}} + }; + + int i, j; + char *orig_line; + smartlist_t *sl; + + (void)ptr; + + for (i=0; tests[i].orig_line; i++) { + sl = smartlist_create(); + orig_line = tor_malloc(tests[i].orig_length); + memcpy(orig_line, tests[i].orig_line, tests[i].orig_length + 1); + tor_split_lines(sl, orig_line, tests[i].orig_length); + + j = 0; + log_info(LD_GENERAL, "Splitting test %d of length %d", + i, tests[i].orig_length); + SMARTLIST_FOREACH(sl, const char *, line, + { + /* Check we have not got too many lines */ + tt_int_op(j, <, MAX_SPLIT_LINE_COUNT); + /* Check that there actually should be a line here */ + tt_assert(tests[i].split_line[j] != NULL); + log_info(LD_GENERAL, "Line %d of test %d, should be <%s>", + j, i, tests[i].split_line[j]); + /* Check that the line is as expected */ + tt_str_op(tests[i].split_line[j], ==, line); + j++; + }); + /* Check that we didn't miss some lines */ + tt_assert(tests[i].split_line[j] == NULL); + tor_free(orig_line); + smartlist_free(sl); + } + + done: + ; +} + static void test_util_di_ops(void) { @@ -1691,6 +1755,7 @@ struct testcase_t util_tests[] = { UTIL_TEST(spawn_background_fail, 0), UTIL_TEST(spawn_background_partial_read, 0), UTIL_TEST(join_cmdline, 0), + UTIL_TEST(split_lines, 0), END_OF_TESTCASES };