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 4d8fc8b91ba234df10edd8d15d552169bcbf9930..77d19f044d71a1fec89ec629877d0deda5b3585d 100644 (file)
--- 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 9e96b984a2a764d5d322fc60e1065e2ee9cb2b6b..3a9a5e86e69319b12cb0a3a7384574f912fed6c6 100644 (file)
--- 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);