Fix bug in alloca_toprint() that caused SEGV

log.c

@@ -306,23 +306,24 @@ unsigned int debugFlagMask(const char *flagname) {
 }
 
 /* Format a buffer of data as a printable representation, eg: "Abc\x0b\n\0", for display
-   in log messages.  If dstStrLen == -1 then assumes the dstStr buffer is large enough to
+   in log messages.
-   hold the representation of the entire srcBuf.
    @author Andrew Bettison <andrew@servalproject.com>
  */
-char *toprint(char *dstStr, ssize_t dstStrLen, const char *srcBuf, size_t srcBytes)
+char *toprint(char *dstStr, ssize_t dstBufSiz, const char *srcBuf, size_t srcBytes)
 {
-  return strbuf_str(strbuf_toprint_quoted_len(strbuf_local(dstStr, (dstStrLen == -1 ? 2 + srcBytes * 4 : dstStrLen) + 1), '"', srcBuf, srcBytes));
+  strbuf b = strbuf_local(dstStr, dstBufSiz);
+  strbuf_toprint_quoted_len(b, '"', srcBuf, srcBytes);
+  return dstStr;
 }
 
-/* Compute the length of the printable string produced by toprint().  If dstStrLen == -1 then
+/* Compute the length of the string produced by toprint().  If dstStrLen == -1 then returns the
-   returns the exact number of characters in the printable representation, otherwise returns
+   exact number of characters in the printable representation (excluding the terminating nul),
-   dstStrLen.
+   otherwise returns dstStrLen.
    @author Andrew Bettison <andrew@servalproject.com>
  */
-size_t toprint_strlen(ssize_t dstStrLen, const char *srcBuf, size_t srcBytes)
+size_t toprint_strlen(const char *srcBuf, size_t srcBytes)
 {
-  return dstStrLen == -1 ? strbuf_count(strbuf_toprint_quoted_len(strbuf_local(NULL, 0), '"', srcBuf, srcBytes)) : dstStrLen;
+  return strbuf_count(strbuf_toprint_quoted_len(strbuf_local(NULL, 0), '"', srcBuf, srcBytes));
 }
 
 /* Read the symbolic link into the supplied buffer and add a terminating nul.  Return -1 if the

log.h

@@ -21,6 +21,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 #include <stdio.h>
 #include <stdarg.h>
+#include "strbuf_helpers.h"
 
 extern unsigned int debug;
 
@@ -69,12 +70,12 @@ void logMessage(int level, const char *file, unsigned int line, const char *func
 void vlogMessage(int level, const char *file, unsigned int line, const char *function, const char *fmt, va_list);
 unsigned int debugFlagMask(const char *flagname);
 int logDump(int level, const char *file, unsigned int line, const char *function, char *name, unsigned char *addr, size_t len);
-char *toprint(char *dstStr, ssize_t dstChars, const char *srcBuf, size_t srcBytes);
+char *toprint(char *dstStr, ssize_t dstBufSiz, const char *srcBuf, size_t srcBytes);
-size_t toprint_strlen(ssize_t dstStrLen, const char *srcBuf, size_t srcBytes);
+size_t toprint_strlen(const char *srcBuf, size_t srcBytes);
 ssize_t get_self_executable_path(char *buf, size_t len);
 int log_backtrace(const char *file, unsigned int line, const char *function);
 
-#define alloca_toprint(dstlen,buf,len)  toprint((char *)alloca(toprint_strlen((dstlen), (buf), (len)) + 1), (dstlen), (buf), (len))
+#define alloca_toprint(dstlen,buf,len)  toprint((char *)alloca((dstlen) == -1 ? toprint_strlen((buf),(len)) + 1 : (dstlen)), (dstlen), (buf), (len))
 
 #define LOGF(L,F,...)       (logMessage(L, __FILE__, __LINE__, __FUNCTION__, F, ##__VA_ARGS__))
 #define LOGF_perror(L,F,...) logMessage_perror(L, __FILE__, __LINE__, __FUNCTION__, F, ##__VA_ARGS__)

strbuf.c

@@ -24,27 +24,25 @@ static inline size_t min(size_t a, size_t b) {
   return a < b ? a : b;
 }
 
-strbuf strbuf_init(strbuf sb, char *buffer, size_t size)
+strbuf strbuf_init(strbuf sb, char *buffer, ssize_t size)
 {
   sb->start = buffer;
-  sb->end = sb->start + size - 1;
+  sb->end = size >= 0 ? sb->start + size - 1 : NULL;
   return strbuf_reset(sb);
 }
 
 strbuf strbuf_reset(strbuf sb)
 {
   sb->current = sb->start;
-  if (sb->start && sb->end >= sb->start) {
+  if (sb->start)
     *sb->start = '\0';
-    *sb->end = '\0'; // should never get overwritten
-  }
   return sb;
 }
 
 strbuf strbuf_ncat(strbuf sb, const char *text, size_t len)
 {
-  if (sb->start && sb->current < sb->end) {
+  if (sb->start && (!sb->end || (sb->current < sb->end))) {
-    register size_t n = min(sb->end - sb->current, len);
+    register size_t n = sb->end ? min(sb->end - sb->current, len) : len;
     char *c;
     for (c = sb->current; n && (*c = *text); --n, ++c, ++text)
       ;
@@ -56,17 +54,15 @@ strbuf strbuf_ncat(strbuf sb, const char *text, size_t len)
 
 strbuf strbuf_puts(strbuf sb, const char *text)
 {
-  if (sb->start && sb->current < sb->end) {
+  if (sb->start && (!sb->end || sb->current < sb->end)) {
-    register size_t n = sb->end - sb->current;
+    register size_t n = sb->end ? sb->end - sb->current : -1;
     while (n-- && (*sb->current = *text)) {
       ++sb->current;
       ++text;
     }
   }
-  while (*text) {
+  while (*text++)
     ++sb->current;
-    ++text;
-  }
   return sb;
 }
 
@@ -76,7 +72,7 @@ strbuf strbuf_tohex(strbuf sb, const unsigned char *data, size_t len)
   char *p = sb->current;
   sb->current += len * 2;
   if (sb->start) {
-    char *e = sb->current < sb->end ? sb->current : sb->end;
+    char *e = sb->end && sb->current > sb->end ? sb->end : sb->current;
     // The following loop could overwrite the '\0' at *sp->end.
     for (; p < e; ++data) {
       *p++ = hexdigit[*data >> 4];
@@ -90,11 +86,11 @@ strbuf strbuf_tohex(strbuf sb, const unsigned char *data, size_t len)
 
 strbuf strbuf_putc(strbuf sb, char ch)
 {
-  if (sb->start && sb->current < sb->end) {
+  if (sb->start && (!sb->end || sb->current < sb->end)) {
-    *sb->current++ = ch;
+    sb->current[0] = ch;
-    *sb->current = '\0';
+    sb->current[1] = '\0';
-  } else
+  }
-    ++sb->current;
+  ++sb->current;
   return sb;
 }
 
@@ -110,9 +106,13 @@ int strbuf_sprintf(strbuf sb, const char *fmt, ...)
 int strbuf_vsprintf(strbuf sb, const char *fmt, va_list ap)
 {
   int n;
-  if (sb->start && sb->current < sb->end) {
+  if (sb->start && !sb->end) {
-    n = vsnprintf(sb->current, sb->end - sb->current + 1, fmt, ap);
+    n = vsprintf(sb->current, fmt, ap);
-    *sb->end = '\0';
+  } else if (sb->start && sb->current < sb->end) {
+    int space = sb->end - sb->current + 1;
+    n = vsnprintf(sb->current, space, fmt, ap);
+    if (n >= space)
+      *sb->end = '\0';
   } else {
     char tmp[1];
     n = vsnprintf(tmp, sizeof tmp, fmt, ap);
@@ -125,13 +125,15 @@ int strbuf_vsprintf(strbuf sb, const char *fmt, va_list ap)
 char *strbuf_substr(const_strbuf sb, int offset)
 {
   char *s;
-  if (offset < 0) {
+  if (!sb->start)
-    s = (sb->current < sb->end ? sb->current : sb->end) + offset;
+    s = NULL;
+  else if (offset < 0) {
+    s = strbuf_end(sb) + offset;
     if (s < sb->start)
       s = sb->start;
   } else {
     s = sb->start + offset;
-    if (s > sb->end)
+    if (sb->end && s > sb->end)
       s = sb->end;
   }
   return s;
@@ -140,14 +142,14 @@ char *strbuf_substr(const_strbuf sb, int offset)
 strbuf strbuf_trunc(strbuf sb, int offset)
 {
   if (offset < 0) {
-    char *e = sb->current < sb->end ? sb->current : sb->end;
+    char *e = strbuf_end(sb);
     sb->current = offset <= sb->start - e ? sb->start : e + offset;
   } else {
     char *s = sb->start + offset;
     if (s < sb->current)
       sb->current = s;
   }
-  if (sb->start && sb->current < sb->end)
+  if (sb->start && (!sb->end || sb->current < sb->end))
     *sb->current = '\0';
   return sb;
 }

strbuf.h

@@ -25,11 +25,10 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
     nul-terminated string in a fixed-size, caller-provided backing buffer,
     using a sequence of append operations.
 
-    An append operation that would overflow the buffer is truncated, and the
+    An append operation that would overflow the buffer is truncated with a nul
-    result nul-terminated.  Once a truncation has occurred, the "overrun"
+    terminator and the "overrun" property of the strbuf becomes true until the
-    property of the strbuf is true until the next strbuf_init(), and all
+    next strbuf_init() or strbuf_trunc().  Any append to an overrun strbuf will
-    subsequent appends will be fully truncated, ie, nothing more will be
+    be fully truncated, ie, nothing more will be appended to the buffer.
-    appended to the buffer.
 
     The string in the buffer is guaranteed to always be nul terminated, which
     means that the maximum strlen() of the assembled string is one less than
@@ -52,9 +51,27 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
     A NULL buffer can be provided.  This causes the strbuf operations to
     perform all character counting and truncation calculations as usual, but
-    not assemble the string.  This allows a strbuf to be used for calculating
+    not actually assemble the string; it is as though the strbuf is permanently
-    the size needed for a buffer, which the caller may then allocate and replay
+    overrun, but no nul terminator is appended.  This allows a strbuf to be
-    the same operations to fill.
+    used for calculating the size needed for a buffer, which the caller may
+    then allocate and replay the same operations to fill.
+
+    A buffer length of -1 can be given.  This causes the strbuf operations to
+    treat the buffer as unlimited in size.  This is useful for when the caller
+    is 100% certain that the strbuf will not be overrun.  For example, if the
+    required buffer size was already computed by a preliminary run of the same
+    strbuf operations on a NULL buffer, and the necessary size allocated.
+
+    The strbuf operations will never write any data beyond the length of the
+    assembled string plus one for the nul terminator.  So, for example, the
+    following code will never alter buf[4]:
+
+    char buf[5];
+    buf[4] = 'x';
+    strbuf b;
+    strbuf_init(b, buf, sizeof buf);
+    strbuf_puts(&b, "abc");
+    assert buf[4] == 'x'; // always passes
 
 */
 
@@ -73,8 +90,8 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 #endif
 
 struct strbuf {
-    char *start;
+    char *start; // NULL after strbuf_init(buffer=NULL)
-    char *end;
+    char *end; // NULL after strbuf_init(size=-1), otherwise end=&start[size-1]
     char *current;
 };
 
@@ -111,7 +128,7 @@ typedef const struct strbuf *const_strbuf;
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-#define strbuf_alloca(size) strbuf_make(alloca(SIZEOF_STRBUF + size), SIZEOF_STRBUF + size)
+#define strbuf_alloca(size) strbuf_make(alloca(SIZEOF_STRBUF + (size)), SIZEOF_STRBUF + (size))
 
 
 /** Convenience macro for filling a strbuf from the calling function's
@@ -175,7 +192,7 @@ typedef const struct strbuf *const_strbuf;
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-strbuf strbuf_init(strbuf sb, char *buffer, size_t size);
+strbuf strbuf_init(strbuf sb, char *buffer, ssize_t size);
 
 
 /** Initialise a strbuf and its backing buffer inside the caller-supplied
@@ -322,7 +339,7 @@ __STRBUF_INLINE char *strbuf_str(const_strbuf sb) {
  * @author Andrew Bettison <andrew@servalproject.com>
  */
 __STRBUF_INLINE char *strbuf_end(const_strbuf sb) {
-  return sb->current < sb->end ? sb->current : sb->end;
+  return sb->end && sb->current > sb->end ? sb->end : sb->current;
 }
 
 
@@ -388,8 +405,8 @@ __STRBUF_INLINE size_t strbuf_is_empty(const_strbuf sb) {
  *
  * @author Andrew Bettison <andrew@servalproject.com>
  */
-__STRBUF_INLINE size_t strbuf_size(const_strbuf sb) {
+__STRBUF_INLINE ssize_t strbuf_size(const_strbuf sb) {
-  return sb->end - sb->start + 1;
+  return sb->end ? sb->end - sb->start + 1 : -1;
 }
 
 
@@ -401,7 +418,7 @@ __STRBUF_INLINE size_t strbuf_size(const_strbuf sb) {
  * @author Andrew Bettison <andrew@servalproject.com>
  */
 __STRBUF_INLINE size_t strbuf_len(const_strbuf sb) {
-  return (sb->current < sb->end ? sb->current : sb->end) - sb->start;
+  return strbuf_end(sb) - sb->start;
 }
 
 
@@ -425,7 +442,7 @@ __STRBUF_INLINE size_t strbuf_count(const_strbuf sb) {
  * @author Andrew Bettison <andrew@servalproject.com>
  */
 __STRBUF_INLINE int strbuf_overrun(const_strbuf sb) {
-  return sb->current > sb->end;
+  return sb->end && sb->current > sb->end;
 }
 
 #define write_str(fd,str)               (_write_str(fd, str, __FILE__, __LINE__, __FUNCTION__))