Merge pull request #955 from carrerasdarren-cell/agent/fix-printbuf-self-append

printbuf: preserve self-append sources across growth

printbuf.c

@@ -28,6 +28,7 @@
 #endif /* HAVE_STDARG_H */
 
 #include "debug.h"
+#include "json_inttypes.h"
 #include "printbuf.h"
 #include "snprintf_compat.h"
 #include "vasprintf_compat.h"
@@ -104,10 +105,19 @@ int printbuf_memappend(struct printbuf *p, const char *buf, int size)
 	}
 	if (p->size <= p->bpos + size + 1)
 	{
+		int buf_offset = -1;
+		const uintptr_t buf_addr = (uintptr_t)(const void *)buf;
+		const uintptr_t p_buf_addr = (uintptr_t)(const void *)p->buf;
+
+		/* realloc() invalidates source pointers into the old buffer. */
+		if (buf_addr >= p_buf_addr && buf_addr - p_buf_addr < (uintptr_t)p->size)
+			buf_offset = (int)(buf_addr - p_buf_addr);
 		if (printbuf_extend(p, p->bpos + size + 1) < 0)
 			return -1;
+		if (buf_offset >= 0)
+			buf = p->buf + buf_offset;
 	}
-	memcpy(p->buf + p->bpos, buf, size);
+	memmove(p->buf + p->bpos, buf, size);
 	p->bpos += size;
 	p->buf[p->bpos] = '\0';
 	return size;

printbuf.h

@@ -52,6 +52,8 @@ JSON_EXPORT struct printbuf *printbuf_new(void);
  *
  * Your code should not use printbuf_memappend() directly unless it
  * checks the return code. Use printbuf_memappend_fast() instead.
+ * The source may point into p->buf, including a region that overlaps the
+ * appended destination.
  */
 JSON_EXPORT int printbuf_memappend(struct printbuf *p, const char *buf, int size);
 
@@ -112,6 +114,8 @@ JSON_EXPORT int printbuf_memset(struct printbuf *pb, int offset, int charvalue,
  * 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.
+ * Format arguments may point into p->buf, including when appending the
+ * formatted result grows the buffer.
  *
  * See also:
  *   printbuf_memappend_fast()

tests/test_printbuf.c

@@ -126,6 +126,59 @@ static void test_printbuf_memappend(int *before_resize)
 	printf("%s: end test\n", __func__);
 }
 
+static void test_printbuf_self_append(void);
+static void test_printbuf_self_append(void)
+{
+	struct printbuf *pb;
+	char *data;
+	int data_size;
+	int i;
+	int initial_size;
+
+	printf("%s: starting test\n", __func__);
+	pb = printbuf_new();
+	assert(pb != NULL);
+	initial_size = pb->size;
+	data_size = initial_size / 2 + 1;
+	data = malloc(data_size);
+	assert(data != NULL);
+	memset(data, 'X', data_size);
+	assert(printbuf_memappend(pb, data, data_size) == data_size);
+	free(data);
+	assert(pb->size == initial_size);
+
+	assert(printbuf_memappend(pb, pb->buf, pb->bpos) == data_size);
+	assert(pb->size > initial_size);
+	assert(pb->bpos == data_size * 2);
+	for (i = 0; i < pb->bpos; i++)
+		assert(pb->buf[i] == 'X');
+	assert(pb->buf[pb->bpos] == '\0');
+
+	printbuf_free(pb);
+
+	/* Formatted arguments may also point into a buffer that must grow. */
+	pb = printbuf_new();
+	assert(pb != NULL);
+	initial_size = pb->size;
+	data_size = initial_size - 2;
+	data = malloc(data_size);
+	assert(data != NULL);
+	memset(data, 'X', data_size);
+	assert(printbuf_memappend(pb, data, data_size) == data_size);
+	free(data);
+	assert(pb->size == initial_size);
+
+	assert(sprintbuf(pb, "%s", pb->buf + 1) == data_size - 1);
+	assert(pb->size > initial_size);
+	assert(pb->bpos == data_size * 2 - 1);
+	for (i = 0; i < pb->bpos; i++)
+		assert(pb->buf[i] == 'X');
+	assert(pb->buf[pb->bpos] == '\0');
+
+	printbuf_free(pb);
+	printf("%s: end test\n", __func__);
+}
+
 static void test_sprintbuf(int before_resize);
 static void test_sprintbuf(int before_resize)
 {
@@ -182,6 +235,8 @@ int main(int argc, char **argv)
 	printf("========================================\n");
 	test_printbuf_memappend(&before_resize);
 	printf("========================================\n");
+	test_printbuf_self_append();
+	printf("========================================\n");
 	test_sprintbuf(before_resize);
 	printf("========================================\n");
 

tests/test_printbuf.expected

@@ -21,6 +21,9 @@ Append to just after resize: 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX]
 Buffer size after printbuf_strappend(): 16, [XXXXXXXXXXXXXXXX]
 test_printbuf_memappend: end test
 ========================================
+test_printbuf_self_append: starting test
+test_printbuf_self_append: end test
+========================================
 test_sprintbuf: starting test
 Buffer length: 0
 sprintbuf to just after resize(31+1): 32, [XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX], strlen(buf)=32