June 2012 - tor-commits - lists.torproject.org

[tor/maint-0.2.3] Merge remote-tracking branch 'public/bug6203_v2' into maint-0.2.3
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit a9de982c343b0d83078d8e46b1a06e483c8595ec Merge: 4a7e412 0600e8c Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 22 22:33:14 2012 -0400 Merge remote-tracking branch 'public/bug6203_v2' into maint-0.2.3 changes/bug6203 | 5 +++++ src/or/main.c | 13 ++++++++++++- 2 files changed, 17 insertions(+), 1 deletions(-)

1 0

[tor/master] Make format_helper_exit_status() avoid unnecessary spaces
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 4c62cc6f9997c215ff2687ce342e00d630d922c7 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Tue Jun 19 04:07:30 2012 -0700 Make format_helper_exit_status() avoid unnecessary spaces --- changes/bug5557 | 3 + src/common/util.c | 100 ++++++++++++++++++++++++++++++++++++++----------- src/test/test_util.c | 10 ++-- 3 files changed, 85 insertions(+), 28 deletions(-) diff --git a/changes/bug5557 b/changes/bug5557 new file mode 100644 index 0000000..c73fbe2 --- /dev/null +++ b/changes/bug5557 @@ -0,0 +1,3 @@ + o Minor bugfixes + - Make format_helper_exit_status() avoid unnecessary space padding and + stop confusing log_from_pipe(). Fixes ticket 5557. diff --git a/src/common/util.c b/src/common/util.c index 28ecff3..63a8aff 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3208,8 +3208,9 @@ void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { - unsigned int unsigned_errno; - char *cur; + unsigned int unsigned_errno, len, tmp_uint; + char *cur, *tmp; + unsigned char tmp_uchar; size_t i; /* Fill hex_errno with spaces, and a trailing newline (memset may @@ -3225,35 +3226,88 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, unsigned_errno = (unsigned int) saved_errno; } - /* Convert errno to hex (start before \n) */ - cur = hex_errno + HEX_ERRNO_SIZE - 2; + /* How many chars do we need for child_state ? */ + if ( child_state > 0 ) { + len = 0; + tmp_uchar = child_state; + while (tmp_uchar > 0) { + tmp_uchar >>= 4; + ++len; + } + } + else len = 1; - /* Check for overflow on first iteration of the loop */ - if (cur < hex_errno) - return; + /* Bail if we would go past the end (on this or the '/') */ + if ( len + 2 > HEX_ERRNO_SIZE) + goto err; + + /* Point to last one */ + cur = hex_errno + len - 1; + /* Convert child_state to hex */ do { - *cur-- = "0123456789ABCDEF"[unsigned_errno % 16]; - unsigned_errno /= 16; - } while (unsigned_errno != 0 && cur >= hex_errno); + *cur-- = "0123456789ABCDEF"[child_state & 0xf]; + child_state >>= 4; + } while (child_state != 0 && cur >= hex_errno); - /* Prepend the minus sign if errno was negative */ - if (saved_errno < 0 && cur >= hex_errno) - *cur-- = '-'; + /* Check for overflow on first iteration of the loop */ + if (cur + 1 < hex_errno) + goto err; - /* Leave a gap */ - if (cur >= hex_errno) - *cur-- = '/'; + /* Now the '/' */ + hex_errno[len] = '/'; - /* Check for overflow on first iteration of the loop */ - if (cur < hex_errno) - return; + /* Save a pointer to the start of second number */ + tmp = hex_errno + len; - /* Convert child_state to hex */ + /* How many chars do we need for unsigned_errno? */ + if ( unsigned_errno > 0 ) { + len = 0; + tmp_uint = unsigned_errno; + while (tmp_uint > 0) { + tmp_uint >>= 4; + ++len; + } + } + else len = 1; + + /* Need minus? */ + if (saved_errno < 0) { + if ( tmp + 1 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + goto err; + + *(++tmp) = '-'; + } + + /* Last check for space */ + if ( tmp + len + 2 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + goto err; + + /* Point to last one */ + cur = tmp + len; + + /* Convert unsigned_errno to hex */ do { - *cur-- = "0123456789ABCDEF"[child_state % 16]; - child_state /= 16; - } while (child_state != 0 && cur >= hex_errno); + *cur-- = "0123456789ABCDEF"[unsigned_errno & 0xf]; + unsigned_errno >>= 4; + } while (unsigned_errno != 0 && cur >= tmp); + + /* Emit the newline and NUL */ + cur = tmp + len; + *(++cur) = '\n'; + *(++cur) = '\0'; + + goto done; + +err: + /* + * In error exit, just write a '\0' in the first char so whatever called + * this at least won't fall off the end. + */ + *hex_errno = '\0'; + +done: + return; } /* Maximum number of file descriptors, if we cannot get it via sysconf() */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 7484b9e..a3a5450 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2143,11 +2143,11 @@ test_util_exit_status(void *ptr) clear_hex_errno(hex_errno); format_helper_exit_status(0, 0, hex_errno); - test_streq(" 0/0\n", hex_errno); + test_streq("0/0\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); - test_streq(" 0/7FFFFFFF\n", hex_errno); + test_streq("0/7FFFFFFF\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0xFF, -0x80000000, hex_errno); @@ -2155,11 +2155,11 @@ test_util_exit_status(void *ptr) clear_hex_errno(hex_errno); format_helper_exit_status(0x7F, 0, hex_errno); - test_streq(" 7F/0\n", hex_errno); + test_streq("7F/0\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0x08, -0x242, hex_errno); - test_streq(" 8/-242\n", hex_errno); + test_streq("8/-242\n", hex_errno); done: ; @@ -2357,7 +2357,7 @@ test_util_spawn_background_fail(void *ptr) tor_snprintf(code, sizeof(code), "%x/%x", 9 /* CHILD_STATE_FAILEXEC */ , ENOENT); tor_snprintf(expected_out, sizeof(expected_out), - "ERR: Failed to spawn background process - code %12s\n", code); + "ERR: Failed to spawn background process - code %s\n", code); run_util_spawn_background(argv, expected_out, expected_err, 255, expected_status);

1 0

[tor/master] Add unit test for format_hex_number_for_helper_exit_status()
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 770374a6b3e6a60b65cf9616755b018f93a17d26 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Wed Jun 20 18:38:07 2012 -0700 Add unit test for format_hex_number_for_helper_exit_status() --- src/test/test_util.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index a3a5450..d71d280 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2465,6 +2465,44 @@ test_util_spawn_background_partial_read(void *ptr) } /** + * Test for format_hex_number_for_helper_exit_status() + */ + +static void +test_util_format_hex_number(void *ptr) +{ + int i, len; + char buf[HEX_ERRNO_SIZE + 1]; + const struct { + const char *str; + unsigned int x; + } test_data[] = { + {"0", 0}, + {"1", 1}, + {"273A", 0x273a}, + {"FFFF", 0xffff}, +#if UINT_MAX >= 0xffffffff + {"31BC421D", 0x31bc421d}, + {"FFFFFFFF", 0xffffffff}, +#endif + {NULL, 0} + }; + + (void)ptr; + + for (i = 0; test_data[i].str != NULL; ++i) { + len = format_hex_number_for_helper_exit_status(test_data[i].x, + buf, HEX_ERRNO_SIZE); + test_neq(len, 0); + buf[len] = '\0'; + test_streq(buf, test_data[i].str); + } + + done: + return; +} + +/** * Test that we can properly format q Windows command line */ static void @@ -3031,6 +3069,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(format_hex_number, 0), UTIL_TEST(join_win_cmdline, 0), UTIL_TEST(split_lines, 0), UTIL_TEST(n_bits_set, 0),

1 0

[tor/master] Refactor unsigned int hex formatting out of format_helper_exit_status() in util.c
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit c21af69f29ca5a55385a926fdac4a8e6c8fb6d37 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Wed Jun 20 14:43:50 2012 -0700 Refactor unsigned int hex formatting out of format_helper_exit_status() in util.c --- src/common/util.c | 154 +++++++++++++++++++++++++++++++++-------------------- src/common/util.h | 2 + 2 files changed, 99 insertions(+), 57 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 63a8aff..79e7a70 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3191,6 +3191,61 @@ tor_join_win_cmdline(const char *argv[]) return joined_argv; } +/** + * Helper function to output hex numbers, called by + * format_helper_exit_status(). This writes the hexadecimal digits of x into + * buf, up to max_len digits, and returns the actual number of digits written. + * If there is insufficient space, it will write nothing and return 0. + * + * This accepts an unsigned int because format_helper_exit_status() needs to + * call it with a signed int and an unsigned char, and since the C standard + * does not guarantee that an int is wider than a char (an int must be at + * least 16 bits but it is permitted for a char to be that wide as well), we + * can't assume a signed int is sufficient to accomodate an unsigned char. + * Thus, format_helper_exit_status() will still need to emit any require '-' + * on its own. + */ + +int +format_hex_number_for_helper_exit_status(unsigned int x, char *buf, + int max_len) +{ + int len; + unsigned int tmp; + char *cur; + + /* Sanity check */ + if (!buf || max_len <= 0) + return 0; + + /* How many chars do we need for x? */ + if (x > 0) { + len = 0; + tmp = x; + while (tmp > 0) { + tmp >>= 4; + ++len; + } + } + else len = 1; + + /* Bail if we would go past the end of the buffer */ + if (len > max_len) + return 0; + + /* Point to last one */ + cur = buf + len - 1; + + /* Convert x to hex */ + do { + *cur-- = "0123456789ABCDEF"[x & 0xf]; + x >>= 4; + } while (x != 0 && cur >= buf); + + /* Return len */ + return len; +} + /** Format child_state and saved_errno as a hex string placed in * hex_errno. Called between fork and _exit, so must be signal-handler * safe. @@ -3208,9 +3263,9 @@ void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { - unsigned int unsigned_errno, len, tmp_uint; - char *cur, *tmp; - unsigned char tmp_uchar; + unsigned int unsigned_errno; + int written, left; + char *cur; size_t i; /* Fill hex_errno with spaces, and a trailing newline (memset may @@ -3226,87 +3281,72 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, unsigned_errno = (unsigned int) saved_errno; } - /* How many chars do we need for child_state ? */ - if ( child_state > 0 ) { - len = 0; - tmp_uchar = child_state; - while (tmp_uchar > 0) { - tmp_uchar >>= 4; - ++len; - } - } - else len = 1; + /* + * Count how many chars of space we have left, and keep a pointer into the + * current point in the buffer. + */ + left = HEX_ERRNO_SIZE; + cur = hex_errno; - /* Bail if we would go past the end (on this or the '/') */ - if ( len + 2 > HEX_ERRNO_SIZE) + /* Emit child_state */ + written = format_hex_number_for_helper_exit_status(child_state, + cur, left); + if (written <= 0) goto err; - /* Point to last one */ - cur = hex_errno + len - 1; - - /* Convert child_state to hex */ - do { - *cur-- = "0123456789ABCDEF"[child_state & 0xf]; - child_state >>= 4; - } while (child_state != 0 && cur >= hex_errno); - - /* Check for overflow on first iteration of the loop */ - if (cur + 1 < hex_errno) + /* Adjust left and cur */ + left -= written; + cur += written; + if (left <= 0) goto err; /* Now the '/' */ - hex_errno[len] = '/'; - - /* Save a pointer to the start of second number */ - tmp = hex_errno + len; + *cur = '/'; - /* How many chars do we need for unsigned_errno? */ - if ( unsigned_errno > 0 ) { - len = 0; - tmp_uint = unsigned_errno; - while (tmp_uint > 0) { - tmp_uint >>= 4; - ++len; - } - } - else len = 1; + /* Adjust left and cur */ + ++cur; + --left; + if (left <= 0) + goto err; /* Need minus? */ if (saved_errno < 0) { - if ( tmp + 1 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + *cur = '-'; + ++cur; + --left; + if (left <= 0) goto err; - - *(++tmp) = '-'; } - /* Last check for space */ - if ( tmp + len + 2 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + /* Emit unsigned_errno */ + written = format_hex_number_for_helper_exit_status(unsigned_errno, + cur, left); + + if (written <= 0) goto err; - /* Point to last one */ - cur = tmp + len; + /* Adjust left and cur */ + left -= written; + cur += written; - /* Convert unsigned_errno to hex */ - do { - *cur-- = "0123456789ABCDEF"[unsigned_errno & 0xf]; - unsigned_errno >>= 4; - } while (unsigned_errno != 0 && cur >= tmp); + /* Check that we have enough space left for a newline */ + if (left <= 0) + goto err; /* Emit the newline and NUL */ - cur = tmp + len; - *(++cur) = '\n'; - *(++cur) = '\0'; + *cur++ = '\n'; + *cur++ = '\0'; goto done; -err: + err: /* * In error exit, just write a '\0' in the first char so whatever called * this at least won't fall off the end. */ *hex_errno = '\0'; -done: + done: return; } diff --git a/src/common/util.h b/src/common/util.h index a2b196c..6b7c6fb 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -471,6 +471,8 @@ void tor_process_handle_destroy(process_handle_t *process_handle, #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */ +int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, + int max_len); void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno);

1 0

[tor/master] Style tweaks and add a warning about NUL-termination
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 4a7e4129af6db990b9672b6d4f013b2e59aa81b7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 22 22:17:24 2012 -0400 Style tweaks and add a warning about NUL-termination --- src/common/util.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 79e7a70..cb8ff85 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3197,6 +3197,9 @@ tor_join_win_cmdline(const char *argv[]) * buf, up to max_len digits, and returns the actual number of digits written. * If there is insufficient space, it will write nothing and return 0. * + * This function DOES NOT add a terminating NUL character to its output: be + * careful! + * * This accepts an unsigned int because format_helper_exit_status() needs to * call it with a signed int and an unsigned char, and since the C standard * does not guarantee that an int is wider than a char (an int must be at @@ -3204,8 +3207,11 @@ tor_join_win_cmdline(const char *argv[]) * can't assume a signed int is sufficient to accomodate an unsigned char. * Thus, format_helper_exit_status() will still need to emit any require '-' * on its own. + * + * For most purposes, you'd want to use tor_snprintf("%x") instead of this + * function; it's designed to be used in code paths where you can't call + * arbitrary C functions. */ - int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, int max_len) @@ -3226,8 +3232,9 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, tmp >>= 4; ++len; } + } else { + len = 1; } - else len = 1; /* Bail if we would go past the end of the buffer */ if (len > max_len) @@ -3258,7 +3265,6 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of * errno when the failure occurred. */ - void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno)

1 0

[tor/master] Merge remote-tracking branch 'origin/maint-0.2.3'
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit a08bbefa9b6d9bf9693ea63e212d7817228e4a3d Merge: f96f319 4a7e412 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 22 22:23:20 2012 -0400 Merge remote-tracking branch 'origin/maint-0.2.3' changes/bug5557 | 3 + src/common/util.c | 148 ++++++++++++++++++++++++++++++++++++++++++-------- src/common/util.h | 2 + src/test/test_util.c | 49 +++++++++++++++-- 4 files changed, 173 insertions(+), 29 deletions(-)

1 0

[tor/maint-0.2.3] Make format_helper_exit_status() avoid unnecessary spaces
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 4c62cc6f9997c215ff2687ce342e00d630d922c7 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Tue Jun 19 04:07:30 2012 -0700 Make format_helper_exit_status() avoid unnecessary spaces --- changes/bug5557 | 3 + src/common/util.c | 100 ++++++++++++++++++++++++++++++++++++++----------- src/test/test_util.c | 10 ++-- 3 files changed, 85 insertions(+), 28 deletions(-) diff --git a/changes/bug5557 b/changes/bug5557 new file mode 100644 index 0000000..c73fbe2 --- /dev/null +++ b/changes/bug5557 @@ -0,0 +1,3 @@ + o Minor bugfixes + - Make format_helper_exit_status() avoid unnecessary space padding and + stop confusing log_from_pipe(). Fixes ticket 5557. diff --git a/src/common/util.c b/src/common/util.c index 28ecff3..63a8aff 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3208,8 +3208,9 @@ void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { - unsigned int unsigned_errno; - char *cur; + unsigned int unsigned_errno, len, tmp_uint; + char *cur, *tmp; + unsigned char tmp_uchar; size_t i; /* Fill hex_errno with spaces, and a trailing newline (memset may @@ -3225,35 +3226,88 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, unsigned_errno = (unsigned int) saved_errno; } - /* Convert errno to hex (start before \n) */ - cur = hex_errno + HEX_ERRNO_SIZE - 2; + /* How many chars do we need for child_state ? */ + if ( child_state > 0 ) { + len = 0; + tmp_uchar = child_state; + while (tmp_uchar > 0) { + tmp_uchar >>= 4; + ++len; + } + } + else len = 1; - /* Check for overflow on first iteration of the loop */ - if (cur < hex_errno) - return; + /* Bail if we would go past the end (on this or the '/') */ + if ( len + 2 > HEX_ERRNO_SIZE) + goto err; + + /* Point to last one */ + cur = hex_errno + len - 1; + /* Convert child_state to hex */ do { - *cur-- = "0123456789ABCDEF"[unsigned_errno % 16]; - unsigned_errno /= 16; - } while (unsigned_errno != 0 && cur >= hex_errno); + *cur-- = "0123456789ABCDEF"[child_state & 0xf]; + child_state >>= 4; + } while (child_state != 0 && cur >= hex_errno); - /* Prepend the minus sign if errno was negative */ - if (saved_errno < 0 && cur >= hex_errno) - *cur-- = '-'; + /* Check for overflow on first iteration of the loop */ + if (cur + 1 < hex_errno) + goto err; - /* Leave a gap */ - if (cur >= hex_errno) - *cur-- = '/'; + /* Now the '/' */ + hex_errno[len] = '/'; - /* Check for overflow on first iteration of the loop */ - if (cur < hex_errno) - return; + /* Save a pointer to the start of second number */ + tmp = hex_errno + len; - /* Convert child_state to hex */ + /* How many chars do we need for unsigned_errno? */ + if ( unsigned_errno > 0 ) { + len = 0; + tmp_uint = unsigned_errno; + while (tmp_uint > 0) { + tmp_uint >>= 4; + ++len; + } + } + else len = 1; + + /* Need minus? */ + if (saved_errno < 0) { + if ( tmp + 1 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + goto err; + + *(++tmp) = '-'; + } + + /* Last check for space */ + if ( tmp + len + 2 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + goto err; + + /* Point to last one */ + cur = tmp + len; + + /* Convert unsigned_errno to hex */ do { - *cur-- = "0123456789ABCDEF"[child_state % 16]; - child_state /= 16; - } while (child_state != 0 && cur >= hex_errno); + *cur-- = "0123456789ABCDEF"[unsigned_errno & 0xf]; + unsigned_errno >>= 4; + } while (unsigned_errno != 0 && cur >= tmp); + + /* Emit the newline and NUL */ + cur = tmp + len; + *(++cur) = '\n'; + *(++cur) = '\0'; + + goto done; + +err: + /* + * In error exit, just write a '\0' in the first char so whatever called + * this at least won't fall off the end. + */ + *hex_errno = '\0'; + +done: + return; } /* Maximum number of file descriptors, if we cannot get it via sysconf() */ diff --git a/src/test/test_util.c b/src/test/test_util.c index 7484b9e..a3a5450 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2143,11 +2143,11 @@ test_util_exit_status(void *ptr) clear_hex_errno(hex_errno); format_helper_exit_status(0, 0, hex_errno); - test_streq(" 0/0\n", hex_errno); + test_streq("0/0\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0, 0x7FFFFFFF, hex_errno); - test_streq(" 0/7FFFFFFF\n", hex_errno); + test_streq("0/7FFFFFFF\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0xFF, -0x80000000, hex_errno); @@ -2155,11 +2155,11 @@ test_util_exit_status(void *ptr) clear_hex_errno(hex_errno); format_helper_exit_status(0x7F, 0, hex_errno); - test_streq(" 7F/0\n", hex_errno); + test_streq("7F/0\n", hex_errno); clear_hex_errno(hex_errno); format_helper_exit_status(0x08, -0x242, hex_errno); - test_streq(" 8/-242\n", hex_errno); + test_streq("8/-242\n", hex_errno); done: ; @@ -2357,7 +2357,7 @@ test_util_spawn_background_fail(void *ptr) tor_snprintf(code, sizeof(code), "%x/%x", 9 /* CHILD_STATE_FAILEXEC */ , ENOENT); tor_snprintf(expected_out, sizeof(expected_out), - "ERR: Failed to spawn background process - code %12s\n", code); + "ERR: Failed to spawn background process - code %s\n", code); run_util_spawn_background(argv, expected_out, expected_err, 255, expected_status);

1 0

[tor/maint-0.2.3] Refactor unsigned int hex formatting out of format_helper_exit_status() in util.c
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit c21af69f29ca5a55385a926fdac4a8e6c8fb6d37 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Wed Jun 20 14:43:50 2012 -0700 Refactor unsigned int hex formatting out of format_helper_exit_status() in util.c --- src/common/util.c | 154 +++++++++++++++++++++++++++++++++-------------------- src/common/util.h | 2 + 2 files changed, 99 insertions(+), 57 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 63a8aff..79e7a70 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3191,6 +3191,61 @@ tor_join_win_cmdline(const char *argv[]) return joined_argv; } +/** + * Helper function to output hex numbers, called by + * format_helper_exit_status(). This writes the hexadecimal digits of x into + * buf, up to max_len digits, and returns the actual number of digits written. + * If there is insufficient space, it will write nothing and return 0. + * + * This accepts an unsigned int because format_helper_exit_status() needs to + * call it with a signed int and an unsigned char, and since the C standard + * does not guarantee that an int is wider than a char (an int must be at + * least 16 bits but it is permitted for a char to be that wide as well), we + * can't assume a signed int is sufficient to accomodate an unsigned char. + * Thus, format_helper_exit_status() will still need to emit any require '-' + * on its own. + */ + +int +format_hex_number_for_helper_exit_status(unsigned int x, char *buf, + int max_len) +{ + int len; + unsigned int tmp; + char *cur; + + /* Sanity check */ + if (!buf || max_len <= 0) + return 0; + + /* How many chars do we need for x? */ + if (x > 0) { + len = 0; + tmp = x; + while (tmp > 0) { + tmp >>= 4; + ++len; + } + } + else len = 1; + + /* Bail if we would go past the end of the buffer */ + if (len > max_len) + return 0; + + /* Point to last one */ + cur = buf + len - 1; + + /* Convert x to hex */ + do { + *cur-- = "0123456789ABCDEF"[x & 0xf]; + x >>= 4; + } while (x != 0 && cur >= buf); + + /* Return len */ + return len; +} + /** Format child_state and saved_errno as a hex string placed in * hex_errno. Called between fork and _exit, so must be signal-handler * safe. @@ -3208,9 +3263,9 @@ void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno) { - unsigned int unsigned_errno, len, tmp_uint; - char *cur, *tmp; - unsigned char tmp_uchar; + unsigned int unsigned_errno; + int written, left; + char *cur; size_t i; /* Fill hex_errno with spaces, and a trailing newline (memset may @@ -3226,87 +3281,72 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, unsigned_errno = (unsigned int) saved_errno; } - /* How many chars do we need for child_state ? */ - if ( child_state > 0 ) { - len = 0; - tmp_uchar = child_state; - while (tmp_uchar > 0) { - tmp_uchar >>= 4; - ++len; - } - } - else len = 1; + /* + * Count how many chars of space we have left, and keep a pointer into the + * current point in the buffer. + */ + left = HEX_ERRNO_SIZE; + cur = hex_errno; - /* Bail if we would go past the end (on this or the '/') */ - if ( len + 2 > HEX_ERRNO_SIZE) + /* Emit child_state */ + written = format_hex_number_for_helper_exit_status(child_state, + cur, left); + if (written <= 0) goto err; - /* Point to last one */ - cur = hex_errno + len - 1; - - /* Convert child_state to hex */ - do { - *cur-- = "0123456789ABCDEF"[child_state & 0xf]; - child_state >>= 4; - } while (child_state != 0 && cur >= hex_errno); - - /* Check for overflow on first iteration of the loop */ - if (cur + 1 < hex_errno) + /* Adjust left and cur */ + left -= written; + cur += written; + if (left <= 0) goto err; /* Now the '/' */ - hex_errno[len] = '/'; - - /* Save a pointer to the start of second number */ - tmp = hex_errno + len; + *cur = '/'; - /* How many chars do we need for unsigned_errno? */ - if ( unsigned_errno > 0 ) { - len = 0; - tmp_uint = unsigned_errno; - while (tmp_uint > 0) { - tmp_uint >>= 4; - ++len; - } - } - else len = 1; + /* Adjust left and cur */ + ++cur; + --left; + if (left <= 0) + goto err; /* Need minus? */ if (saved_errno < 0) { - if ( tmp + 1 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + *cur = '-'; + ++cur; + --left; + if (left <= 0) goto err; - - *(++tmp) = '-'; } - /* Last check for space */ - if ( tmp + len + 2 - hex_errno > (ptrdiff_t)(HEX_ERRNO_SIZE) ) + /* Emit unsigned_errno */ + written = format_hex_number_for_helper_exit_status(unsigned_errno, + cur, left); + + if (written <= 0) goto err; - /* Point to last one */ - cur = tmp + len; + /* Adjust left and cur */ + left -= written; + cur += written; - /* Convert unsigned_errno to hex */ - do { - *cur-- = "0123456789ABCDEF"[unsigned_errno & 0xf]; - unsigned_errno >>= 4; - } while (unsigned_errno != 0 && cur >= tmp); + /* Check that we have enough space left for a newline */ + if (left <= 0) + goto err; /* Emit the newline and NUL */ - cur = tmp + len; - *(++cur) = '\n'; - *(++cur) = '\0'; + *cur++ = '\n'; + *cur++ = '\0'; goto done; -err: + err: /* * In error exit, just write a '\0' in the first char so whatever called * this at least won't fall off the end. */ *hex_errno = '\0'; -done: + done: return; } diff --git a/src/common/util.h b/src/common/util.h index a2b196c..6b7c6fb 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -471,6 +471,8 @@ void tor_process_handle_destroy(process_handle_t *process_handle, #ifdef UTIL_PRIVATE /* Prototypes for private functions only used by util.c (and unit tests) */ +int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, + int max_len); void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno);

1 0

[tor/maint-0.2.3] Add unit test for format_hex_number_for_helper_exit_status()
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 770374a6b3e6a60b65cf9616755b018f93a17d26 Author: Andrea Shepard <andrea(a)persephoneslair.org> Date: Wed Jun 20 18:38:07 2012 -0700 Add unit test for format_hex_number_for_helper_exit_status() --- src/test/test_util.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 files changed, 39 insertions(+), 0 deletions(-) diff --git a/src/test/test_util.c b/src/test/test_util.c index a3a5450..d71d280 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2465,6 +2465,44 @@ test_util_spawn_background_partial_read(void *ptr) } /** + * Test for format_hex_number_for_helper_exit_status() + */ + +static void +test_util_format_hex_number(void *ptr) +{ + int i, len; + char buf[HEX_ERRNO_SIZE + 1]; + const struct { + const char *str; + unsigned int x; + } test_data[] = { + {"0", 0}, + {"1", 1}, + {"273A", 0x273a}, + {"FFFF", 0xffff}, +#if UINT_MAX >= 0xffffffff + {"31BC421D", 0x31bc421d}, + {"FFFFFFFF", 0xffffffff}, +#endif + {NULL, 0} + }; + + (void)ptr; + + for (i = 0; test_data[i].str != NULL; ++i) { + len = format_hex_number_for_helper_exit_status(test_data[i].x, + buf, HEX_ERRNO_SIZE); + test_neq(len, 0); + buf[len] = '\0'; + test_streq(buf, test_data[i].str); + } + + done: + return; +} + +/** * Test that we can properly format q Windows command line */ static void @@ -3031,6 +3069,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(format_hex_number, 0), UTIL_TEST(join_win_cmdline, 0), UTIL_TEST(split_lines, 0), UTIL_TEST(n_bits_set, 0),

1 0

[tor/maint-0.2.3] Style tweaks and add a warning about NUL-termination
by nickm＠torproject.org 23 Jun '12

23 Jun '12

commit 4a7e4129af6db990b9672b6d4f013b2e59aa81b7 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Jun 22 22:17:24 2012 -0400 Style tweaks and add a warning about NUL-termination --- src/common/util.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index 79e7a70..cb8ff85 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3197,6 +3197,9 @@ tor_join_win_cmdline(const char *argv[]) * buf, up to max_len digits, and returns the actual number of digits written. * If there is insufficient space, it will write nothing and return 0. * + * This function DOES NOT add a terminating NUL character to its output: be + * careful! + * * This accepts an unsigned int because format_helper_exit_status() needs to * call it with a signed int and an unsigned char, and since the C standard * does not guarantee that an int is wider than a char (an int must be at @@ -3204,8 +3207,11 @@ tor_join_win_cmdline(const char *argv[]) * can't assume a signed int is sufficient to accomodate an unsigned char. * Thus, format_helper_exit_status() will still need to emit any require '-' * on its own. + * + * For most purposes, you'd want to use tor_snprintf("%x") instead of this + * function; it's designed to be used in code paths where you can't call + * arbitrary C functions. */ - int format_hex_number_for_helper_exit_status(unsigned int x, char *buf, int max_len) @@ -3226,8 +3232,9 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, tmp >>= 4; ++len; } + } else { + len = 1; } - else len = 1; /* Bail if we would go past the end of the buffer */ if (len > max_len) @@ -3258,7 +3265,6 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf, * CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of * errno when the failure occurred. */ - void format_helper_exit_status(unsigned char child_state, int saved_errno, char *hex_errno)

1 0