Restore sprintbuf(), add macro for string literals

Hawciz pointed out that the previous commit modifies the public
interface of printbuf. Per his suggestion, sprintbuf() was restored
and a new pair of macros was added that wraps printbuf_memappend().

Using a wrapper macro instead of modifying sprintbuf() also reduces
function call overhead, bringing total performance gains to
approximately 400%.

json_object.c

@@ -149,11 +149,10 @@ static int json_escape_str(struct printbuf *pb, const char *str, int len, int fl
 							   pos - start_offset);
 				static char sbuf[7];
 				snprintf(sbuf, sizeof(sbuf),
-                                        "\\u00%c%c",
-                                        json_hex_chars[c >> 4],
-                                        json_hex_chars[c & 0xf]);
-				printbuf_memappend (pb, sbuf, sizeof(sbuf) - 1);
-
+					 "\\u00%c%c",
+					 json_hex_chars[c >> 4],
+					 json_hex_chars[c & 0xf]);
+				printbuf_memappend_fast(pb, sbuf, (int) sizeof(sbuf) - 1);
 				start_offset = ++pos;
 			} else
 				pos++;
@@ -364,29 +363,29 @@ static int json_object_object_to_json_string(struct json_object* jso,
 	int had_children = 0;
 	struct json_object_iter iter;
 
-	sprintbuf(pb, "{" /*}*/);
+	printbuf_strappend(pb, "{" /*}*/);
 	if (flags & JSON_C_TO_STRING_PRETTY)
-		sprintbuf(pb, "\n");
+		printbuf_strappend(pb, "\n");
 	json_object_object_foreachC(jso, iter)
 	{
 		if (had_children)
 		{
-			sprintbuf(pb, ",");
+			printbuf_strappend(pb, ",");
 			if (flags & JSON_C_TO_STRING_PRETTY)
-				sprintbuf(pb, "\n");
+				printbuf_strappend(pb, "\n");
 		}
 		had_children = 1;
 		if (flags & JSON_C_TO_STRING_SPACED)
-			sprintbuf(pb, " ");
+			printbuf_strappend(pb, " ");
 		indent(pb, level+1, flags);
-		sprintbuf(pb, "\"");
+		printbuf_strappend(pb, "\"");
 		json_escape_str(pb, iter.key, strlen(iter.key), flags);
 		if (flags & JSON_C_TO_STRING_SPACED)
-			sprintbuf(pb, "\": ");
+			printbuf_strappend(pb, "\": ");
 		else
-			sprintbuf(pb, "\":");
+			printbuf_strappend(pb, "\":");
 		if(iter.val == NULL)
-			sprintbuf(pb, "null");
+			printbuf_strappend(pb, "null");
 		else
 			if (iter.val->_to_json_string(iter.val, pb, level+1,flags) < 0)
 				return -1;
@@ -394,13 +393,13 @@ static int json_object_object_to_json_string(struct json_object* jso,
 	if (flags & JSON_C_TO_STRING_PRETTY)
 	{
 		if (had_children)
-			sprintbuf(pb, "\n");
+			printbuf_strappend(pb, "\n");
 		indent(pb,level,flags);
 	}
 	if (flags & JSON_C_TO_STRING_SPACED)
-		return sprintbuf(pb, /*{*/ " }");
+		return printbuf_strappend(pb, /*{*/ " }");
 	else
-		return sprintbuf(pb, /*{*/ "}");
+		return printbuf_strappend(pb, /*{*/ "}");
 }
 
 
@@ -541,8 +540,8 @@ static int json_object_boolean_to_json_string(struct json_object* jso,
 					      int flags)
 {
 	if (jso->o.c_boolean)
-		return sprintbuf(pb, "true");
-	return sprintbuf(pb, "false");
+		return printbuf_strappend(pb, "true");
+	return printbuf_strappend(pb, "false");
 }
 
 struct json_object* json_object_new_boolean(json_bool b)
@@ -592,7 +591,7 @@ static int json_object_int_to_json_string(struct json_object* jso,
 	/* room for 19 digits, the sign char, and a null term */
 	static char sbuf[21];
 	snprintf(sbuf, sizeof(sbuf), "%"PRId64, jso->o.c_int64);
-	return sprintbuf(pb, sbuf);
+	return printbuf_memappend (pb, sbuf, strlen(sbuf));
 }
 
 struct json_object* json_object_new_int(int32_t i)
@@ -860,9 +859,9 @@ static int json_object_string_to_json_string(struct json_object* jso,
 					     int level,
 					     int flags)
 {
-	sprintbuf(pb, "\"");
+	printbuf_strappend(pb, "\"");
 	json_escape_str(pb, get_string_component(jso), jso->o.c_string.len, flags);
-	sprintbuf(pb, "\"");
+	printbuf_strappend(pb, "\"");
 	return 0;
 }
 
@@ -979,25 +978,25 @@ static int json_object_array_to_json_string(struct json_object* jso,
 	int had_children = 0;
 	size_t ii;
 
-	sprintbuf(pb, "[");
+	printbuf_strappend(pb, "[");
 	if (flags & JSON_C_TO_STRING_PRETTY)
-		sprintbuf(pb, "\n");
+		printbuf_strappend(pb, "\n");
 	for(ii=0; ii < json_object_array_length(jso); ii++)
 	{
 		struct json_object *val;
 		if (had_children)
 		{
-			sprintbuf(pb, ",");
+			printbuf_strappend(pb, ",");
 			if (flags & JSON_C_TO_STRING_PRETTY)
-				sprintbuf(pb, "\n");
+				printbuf_strappend(pb, "\n");
 		}
 		had_children = 1;
 		if (flags & JSON_C_TO_STRING_SPACED)
-			sprintbuf(pb, " ");
+			printbuf_strappend(pb, " ");
 		indent(pb, level + 1, flags);
 		val = json_object_array_get_idx(jso, ii);
 		if(val == NULL)
-			sprintbuf(pb, "null");
+			printbuf_strappend(pb, "null");
 		else
 			if (val->_to_json_string(val, pb, level+1, flags) < 0)
 				return -1;
@@ -1005,13 +1004,13 @@ static int json_object_array_to_json_string(struct json_object* jso,
 	if (flags & JSON_C_TO_STRING_PRETTY)
 	{
 		if (had_children)
-			sprintbuf(pb, "\n");
+			printbuf_strappend(pb, "\n");
 		indent(pb,level,flags);
 	}
 
 	if (flags & JSON_C_TO_STRING_SPACED)
-		return sprintbuf(pb, " ]");
-	return sprintbuf(pb, "]");
+		return printbuf_strappend(pb, " ]");
+	return printbuf_strappend(pb, "]");
 }
 
 static void json_object_array_entry_free(void *data)

printbuf.c

@@ -85,7 +85,6 @@ int printbuf_memappend(struct printbuf *p, const char *buf, int size)
     if (printbuf_extend(p, p->bpos + size + 1) < 0)
       return -1;
   }
-
   memcpy(p->buf + p->bpos, buf, size);
   p->bpos += size;
   p->buf[p->bpos]= '\0';
@@ -112,6 +111,34 @@ int printbuf_memset(struct printbuf *pb, int offset, int charvalue, int len)
 	return 0;
 }
 
+int sprintbuf(struct printbuf *p, const char *msg, ...)
+{
+  va_list ap;
+  char *t;
+  int size;
+  char buf[128];
+
+  /* user stack buffer first */
+  va_start(ap, msg);
+  size = vsnprintf(buf, 128, msg, ap);
+  va_end(ap);
+  /* if string is greater than stack buffer, then use dynamic string
+     with vasprintf.  Note: some implementation of vsnprintf return -1
+     if output is truncated whereas some return the number of bytes that
+     would have been written - this code handles both cases. */
+  if(size == -1 || size > 127) {
+    va_start(ap, msg);
+    if((size = vasprintf(&t, msg, ap)) < 0) { va_end(ap); return -1; }
+    va_end(ap);
+    printbuf_memappend(p, t, size);
+    free(t);
+    return size;
+  } else {
+    printbuf_memappend(p, buf, size);
+    return size;
+  }
+}
+
 void printbuf_reset(struct printbuf *p)
 {
   p->buf[0] = '\0';
@@ -125,8 +152,3 @@ void printbuf_free(struct printbuf *p)
     free(p);
   }
 }
-
-inline int sprintbuf(struct printbuf *p, const char *buf)
-{
-  return printbuf_memappend(p, buf, strlen(buf));
-}

printbuf.h

@@ -29,12 +29,13 @@ struct printbuf {
 extern struct printbuf*
 printbuf_new(void);
 
-/* As an optimization, printbuf_memappend_fast is defined as a macro
+/* As an optimization, printbuf_memappend_fast() is defined as a macro
  * that handles copying data if the buffer is large enough; otherwise
- * it invokes printbuf_memappend_real() which performs the heavy
+ * it invokes printbuf_memappend() which performs the heavy
  * lifting of realloc()ing the buffer and copying data.
- * Your code should not use printbuf_memappend directly--use
- * printbuf_memappend_fast instead.
+ *
+ * Your code should not use printbuf_memappend() directly unless it
+ * checks the return code. Use printbuf_memappend_fast() instead.
  */
 extern int
 printbuf_memappend(struct printbuf *p, const char *buf, int size);
@@ -50,6 +51,28 @@ do {                                                         \
 
 #define printbuf_length(p) ((p)->bpos)
 
+/**
+ * Results in a compile error if the argument is not a string literal.
+ */
+#define _printbuf_check_literal(mystr) ("" mystr)
+
+/**
+ * This is an optimization wrapper around printbuf_memappend() that is useful
+ * for appending string literals. Since the size of string constants is known
+ * at compile time, using this macro can avoid a costly strlen() call. This is
+ * especially helpful when a constant string must be appended many times. If
+ * you got here because of a compilation error caused by passing something
+ * other than a string literal, use printbuf_memappend_fast() in conjunction
+ * with strlen().
+ *
+ * See also:
+ *   printbuf_memappend_fast()
+ *   printbuf_memappend()
+ *   sprintbuf()
+ */
+#define printbuf_strappend(pb, str) \
+   printbuf_memappend ((pb), _printbuf_check_literal(str), sizeof(str) - 1)
+
 /**
  * Set len bytes of the buffer to charvalue, starting at offset offset.
  * Similar to calling memset(x, charvalue, len);
@@ -61,8 +84,22 @@ do {                                                         \
 extern int
 printbuf_memset(struct printbuf *pb, int offset, int charvalue, int len);
 
+/**
+ * Formatted print to printbuf.
+ *
+ * This function is the most expensive of the available functions for appending
+ * string data to a printbuf and should be used only where convenience is more
+ * important than speed. Avoid using this function in high performance code or
+ * tight loops; in these scenarios, consider using snprintf() with a static
+ * buffer in conjunction with one of the printbuf_*append() functions.
+ *
+ * See also:
+ *   printbuf_memappend_fast()
+ *   printbuf_memappend()
+ *   printbuf_strappend()
+ */
 extern int
-sprintbuf(struct printbuf *p, const char *msg);
+sprintbuf(struct printbuf *p, const char *msg, ...);
 
 extern void
 printbuf_reset(struct printbuf *p);

tests/test_printbuf.c

@@ -17,7 +17,7 @@ static void test_basic_printbuf_memset()
 
 	printf("%s: starting test\n", __func__);
 	pb = printbuf_new();
-	sprintbuf(pb, "blue:1");
+	sprintbuf(pb, "blue:%d", 1);
 	printbuf_memset(pb, -1, 'x', 52);
 	printf("Buffer contents:%.*s\n", printbuf_length(pb), pb->buf);
 	printbuf_free(pb);
@@ -106,6 +106,49 @@ static void test_printbuf_memappend(int *before_resize)
 	printf("Append to just after resize: %d, [%s]\n", printbuf_length(pb), pb->buf);
 
 	free(data);
+	printbuf_free(pb);
+
+#define SA_TEST_STR "XXXXXXXXXXXXXXXX"
+	pb = printbuf_new();
+	printbuf_strappend(pb, SA_TEST_STR);
+	printf("Buffer size after printbuf_strappend(): %d, [%s]\n", printbuf_length(pb), pb->buf);
+	printbuf_free(pb);
+#undef  SA_TEST_STR
+
+	printf("%s: end test\n", __func__);
+}
+
+static void test_sprintbuf(int before_resize);
+static void test_sprintbuf(int before_resize)
+{
+	struct printbuf *pb;
+
+	printf("%s: starting test\n", __func__);
+	pb = printbuf_new();
+	printf("Buffer length: %d\n", printbuf_length(pb));
+
+	char *data = malloc(before_resize + 1 + 1);
+	memset(data, 'X', before_resize + 1 + 1);
+	data[before_resize + 1] = '\0';
+	sprintbuf(pb, "%s", data);
+	free(data);
+	printf("sprintbuf to just after resize(%d+1): %d, [%s], strlen(buf)=%d\n", before_resize, printbuf_length(pb), pb->buf, (int)strlen(pb->buf));
+
+	printbuf_reset(pb);
+	sprintbuf(pb, "plain");
+	printf("%d, [%s]\n", printbuf_length(pb), pb->buf);
+
+	sprintbuf(pb, "%d", 1);
+	printf("%d, [%s]\n", printbuf_length(pb), pb->buf);
+
+	sprintbuf(pb, "%d", INT_MAX);
+	printf("%d, [%s]\n", printbuf_length(pb), pb->buf);
+
+	sprintbuf(pb, "%d", INT_MIN);
+	printf("%d, [%s]\n", printbuf_length(pb), pb->buf);
+
+	sprintbuf(pb, "%s", "%s");
+	printf("%d, [%s]\n", printbuf_length(pb), pb->buf);
 
 	printbuf_free(pb);
 	printf("%s: end test\n", __func__);
@@ -123,6 +166,8 @@ int main(int argc, char **argv)
 	printf("========================================\n");
 	test_printbuf_memappend(&before_resize);
 	printf("========================================\n");
+	test_sprintbuf(before_resize);
+	printf("========================================\n");
 
 	return 0;
 }

tests/test_printbuf.expected

@@ -18,5 +18,16 @@ Partial append: 3, [blu]
 With embedded \0 character: 4, [ab]
 Append to just before resize: 31, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX]
 Append to just after resize: 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX]
+Buffer size after printbuf_strappend(): 16, [XXXXXXXXXXXXXXXX]
 test_printbuf_memappend: end test
 ========================================
+test_sprintbuf: starting test
+Buffer length: 0
+sprintbuf to just after resize(31+1): 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX], strlen(buf)=32
+5, [plain]
+6, [plain1]
+16, [plain12147483647]
+27, [plain12147483647-2147483648]
+29, [plain12147483647-2147483648%s]
+test_sprintbuf: end test
+========================================