commit 4c62cc6f9997c215ff2687ce342e00d630d922c7 Author: Andrea Shepard andrea@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);