From c3271a967dc7e221b657cbf420882b605d5c261e Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Thu, 28 Nov 2013 17:34:56 +0100 Subject: [PATCH] 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. --- src/utils/strbuf.c | 17 +++++++++++++---- t/utils/strbuf_test.c | 24 ++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) 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); -- 2.30.2