commit 9fda7e8cd1bbc33479c667ea97a220333f81c148 Author: Nick Mathewson <nickm@torproject.org> Date: Mon Jul 15 12:52:29 2013 -0400 Lightly refactor and test format_hex_number_sigsafe Better tests for upper bounds, and for failing cases. Also, change the function's interface to take a buffer length rather than a maximum length, and then NUL-terminate: functions that don't NUL-terminate are trouble waiting to happen. --- src/common/util.c | 20 +++++++++++--------- src/test/test_util.c | 14 ++++++++++---- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/common/util.c b/src/common/util.c index a5e41bf..d9913dd 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3382,13 +3382,13 @@ tor_join_win_cmdline(const char *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. + * Helper function to output hex numbers from within a signal handler. * - * This function DOES NOT add a terminating NUL character to its output: be - * careful! + * Writes the nul-terminated hexadecimal digits of <b>x</b> into a buffer + * <b>buf</b> of size <b>buf_len</b>, and return the actual number of digits + * written, not counting the terminal NUL. + * + * If there is insufficient space, 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 @@ -3403,14 +3403,14 @@ tor_join_win_cmdline(const char *argv[]) * arbitrary C functions. */ int -format_hex_number_sigsafe(unsigned int x, char *buf, int max_len) +format_hex_number_sigsafe(unsigned int x, char *buf, int buf_len) { int len; unsigned int tmp; char *cur; /* Sanity check */ - if (!buf || max_len <= 0) + if (!buf || buf_len <= 1) return 0; /* How many chars do we need for x? */ @@ -3426,7 +3426,7 @@ format_hex_number_sigsafe(unsigned int x, char *buf, int max_len) } /* Bail if we would go past the end of the buffer */ - if (len > max_len) + if (len+1 > buf_len) return 0; /* Point to last one */ @@ -3438,6 +3438,8 @@ format_hex_number_sigsafe(unsigned int x, char *buf, int max_len) x >>= 4; } while (x != 0 && cur >= buf); + buf[len] = '\0'; + /* Return len */ return len; } diff --git a/src/test/test_util.c b/src/test/test_util.c index 4880b73..2cc25e9 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -2643,7 +2643,8 @@ test_util_format_hex_number(void *ptr) {"1", 1}, {"273A", 0x273a}, {"FFFF", 0xffff}, - + {"7FFFFFFF", 0x7fffffff}, + {"FFFFFFFF", 0xffffffff}, #if UINT_MAX >= 0xffffffff {"31BC421D", 0x31bc421d}, {"FFFFFFFF", 0xffffffff}, @@ -2654,18 +2655,23 @@ test_util_format_hex_number(void *ptr) (void)ptr; for (i = 0; test_data[i].str != NULL; ++i) { - len = format_hex_number_sigsafe(test_data[i].x, buf, 32); + len = format_hex_number_sigsafe(test_data[i].x, buf, sizeof(buf)); test_neq(len, 0); - buf[len] = '\0'; + test_eq(len, strlen(buf)); test_streq(buf, test_data[i].str); } + test_eq(4, format_hex_number_sigsafe(0xffff, buf, 5)); + test_streq(buf, "FFFF"); + test_eq(0, format_hex_number_sigsafe(0xffff, buf, 4)); + test_eq(0, format_hex_number_sigsafe(0, buf, 1)); + done: return; } /** - * Test that we can properly format q Windows command line + * Test that we can properly format a Windows command line */ static void test_util_join_win_cmdline(void *ptr)