Refactor unsigned int hex formatting out of format_helper_exit_status() in util.c

2 files changed, 99 insertions, 57 deletions

diff --git a/src/common/util.c b/src/common/util.c
index 63a8aff404..79e7a70417 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 <b>child_state</b> and <b>saved_errno</b> as a hex string placed in
  * <b>hex_errno</b>.  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 a2b196c88b..6b7c6fb623 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);