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.

diff --git a/src/common/util.c b/src/common/util.c
index a5e41bf256f2c06e742566b054aa515fc120bb7b..d9913dda45cc70936ec2be78a38a876105cabdcf 100644 (file)
--- 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 4880b73a55fb3881cfd2839b9d95dd64f841a50a..2cc25e9861ed490020ed7939688e1fd4d1aa46e6 100644 (file)
--- 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)