From: Sebastian Harl <sh@tokkee.org>
Date: Thu, 28 Nov 2013 16:34:56 +0000 (+0100)
Subject: utils strbuf: Handle nul-byte correctly.
X-Git-Tag: sysdb-0.1.0~336^2~6
X-Git-Url: https://git.tokkee.org/?a=commitdiff_plain;h=c3271a967dc7e221b657cbf420882b605d5c261e;p=sysdb.git

utils strbuf: Handle nul-byte correctly.

A corner-case was not handled correctly before: when writing just enough
characters to the buffer that it would then be full, a resize was attempted
without changing the size of the buffer, leading to an endless loop in
vappend(). This was caused by not taking into account the space required for
the trailing nul-byte.

A test-case which exposes this problem has also been added.
---

diff --git a/src/utils/strbuf.c b/src/utils/strbuf.c
index 4d8fc8b..77d19f0 100644
--- a/src/utils/strbuf.c
+++ b/src/utils/strbuf.c
@@ -119,9 +119,14 @@ sdb_strbuf_vappend(sdb_strbuf_t *strbuf, const char *fmt, va_list ap)
 
 	assert((strbuf->size == 0) || (strbuf->string[strbuf->pos] == '\0'));
 
-	if (strbuf->pos >= strbuf->size)
+	if (! strbuf->size) {
 		/* use some arbitrary but somewhat reasonable default */
-		if (strbuf_resize(strbuf, strbuf->size ? 2 * strbuf->size : 64))
+		if (strbuf_resize(strbuf, 64))
+			return -1;
+	}
+	/* make sure to reserve space for the nul-byte */
+	else if (strbuf->pos >= strbuf->size - 1)
+		if (strbuf_resize(strbuf, 2 * strbuf->size))
 			return -1;
 
 	assert(strbuf->size && strbuf->string);
@@ -137,8 +142,12 @@ sdb_strbuf_vappend(sdb_strbuf_t *strbuf, const char *fmt, va_list ap)
 		return status;
 	}
 
-	if ((size_t)status >= strbuf->size - strbuf->pos) {
-		strbuf_resize(strbuf, (size_t)status + 1);
+	/* 'status' does not include nul-byte */
+	if ((size_t)status >= strbuf->size - strbuf->pos - 1) {
+		if (strbuf_resize(strbuf, strbuf->size + (size_t)status)) {
+			va_end(aq);
+			return -1;
+		}
 
 		/* reset string and try again */
 		strbuf->string[strbuf->pos] = '\0';
diff --git a/t/utils/strbuf_test.c b/t/utils/strbuf_test.c
index 9e96b98..3a9a5e8 100644
--- a/t/utils/strbuf_test.c
+++ b/t/utils/strbuf_test.c
@@ -207,6 +207,29 @@ START_TEST(test_sdb_strbuf_sprintf)
 }
 END_TEST
 
+START_TEST(test_incremental)
+{
+	ssize_t n;
+	size_t i;
+
+	sdb_strbuf_destroy(buf);
+	buf = sdb_strbuf_create(1024);
+
+	/* fill buffer one by one; leave room for nul-byte */
+	for (i = 0; i < 1023; ++i) {
+		n = sdb_strbuf_append(buf, ".");
+		fail_unless(n == 1, "sdb_strbuf_append() = %zi; expected: 1", n);
+	}
+
+	/* write another byte; this has to trigger a resize */
+	n = sdb_strbuf_append(buf, ".");
+	fail_unless(n == 1, "sdb_strbuf_append() = %zi; expected: 1", n);
+
+	n = (ssize_t)sdb_strbuf_len(buf);
+	fail_unless(n == 1024, "sdb_strbuf_len() = %zi; expectd: 1024", n);
+}
+END_TEST
+
 static struct {
 	const char *input;
 	size_t size;
@@ -448,6 +471,7 @@ util_strbuf_suite(void)
 	tcase_add_test(tc, test_sdb_strbuf_create);
 	tcase_add_test(tc, test_sdb_strbuf_append);
 	tcase_add_test(tc, test_sdb_strbuf_sprintf);
+	tcase_add_test(tc, test_incremental);
 	tcase_add_test(tc, test_sdb_strbuf_memcpy);
 	tcase_add_test(tc, test_sdb_strbuf_memappend);
 	tcase_add_test(tc, test_sdb_strbuf_chomp);