From 66f80ae9a0a43cb4d8eac188eb73114829fdcb42 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 26 Oct 2010 21:12:05 +0100 Subject: Change Dynstr to not allocate memory for string separately. Suggested by Richard Braakman. --- dynstr.c | 101 ++++++++++++++++++++++++++++++------------------------------ unittests.c | 32 +------------------ 2 files changed, 52 insertions(+), 81 deletions(-) diff --git a/dynstr.c b/dynstr.c index 40bec34..47cfad3 100644 --- a/dynstr.c +++ b/dynstr.c @@ -34,9 +34,8 @@ /* This is how the dynamic strings are represented. */ struct Dynstr { - const unsigned char *mem; + unsigned char *mem; size_t size; - bool dynamic; }; @@ -102,38 +101,51 @@ static void *alloc(size_t size) } -/* Allocate a new dynamic string. If dynamic is true, the contents of the - * new string is copied from mem. Otherwise the new string just uses mem - * directly. */ -static Dynstr *newstr(const void *mem, size_t size, bool dynamic) +static Dynstr *new_dynstr(const void *mem, size_t size, bool dynamic) { Dynstr *dynstr; - dynstr = alloc(sizeof(Dynstr)); - if (dynstr == NULL) - return NULL; if (dynamic) { - void *newmem = alloc(size); - if (newmem == NULL) { - free(dynstr); + dynstr = alloc(sizeof(Dynstr) + size); + if (dynstr == NULL) return NULL; - } - memcpy(newmem, mem, size); - dynstr->mem = newmem; - dynstr->size = size; - dynstr->dynamic = true; + dynstr->mem = ((unsigned char *) dynstr) + sizeof(Dynstr); } else { - dynstr->mem = mem; - dynstr->size = size; - dynstr->dynamic = false; + dynstr = alloc(sizeof(Dynstr)); + if (dynstr == NULL) + return NULL; + dynstr->mem = (unsigned char *) mem; } + + dynstr->size = size; + return dynstr; +} + + +static void init_dynstr(Dynstr *dynstr, const void *mem, bool dynamic) +{ + if (dynamic) + memcpy(dynstr->mem, mem, dynstr->size); +} + + +/* Allocate a new dynamic string. If dynamic is true, the contents of the + * new string is copied from mem. Otherwise the new string just uses mem + * directly. */ +static Dynstr *newstr(const void *mem, size_t size, bool dynamic) +{ + Dynstr *dynstr; + + dynstr = new_dynstr(mem, size, dynamic); + if (dynstr != NULL) + init_dynstr(dynstr, mem, dynamic); return dynstr; } Dynstr *dynstr_new_empty(void) { - return newstr(NULL, 0, true); + return new_dynstr(NULL, 0, false); } @@ -163,11 +175,7 @@ Dynstr *dynstr_new_from_constant_memory(const void *mem, size_t size) void dynstr_free(Dynstr *dynstr) { - if (dynstr) { - if (dynstr->dynamic) - free((void *) dynstr->mem); - free(dynstr); - } + free(dynstr); } @@ -233,7 +241,6 @@ Dynstr *dynstr_cat_many(Dynstr *dynstr, ...) va_list args; Dynstr *result; size_t size; - void *mem; size_t offset; va_start(args, dynstr); @@ -242,25 +249,18 @@ Dynstr *dynstr_cat_many(Dynstr *dynstr, ...) size += p->size; va_end(args); - mem = alloc(size); - if (mem == NULL) + result = new_dynstr(NULL, size, true); + if (result == NULL) return NULL; va_start(args, dynstr); offset = 0; for (Dynstr *p = dynstr; p != NULL; p = va_arg(args, Dynstr *)) { - memcpy(mem + offset, p->mem, p->size); + memcpy(result->mem + offset, p->mem, p->size); offset += p->size; } va_end(args); - result = newstr(mem, size, false); - if (result == NULL) { - free(mem); - return NULL; - } - result->dynamic = true; - return result; } @@ -374,29 +374,30 @@ static Dynstr *read_helper(size_t (*callback)(FILE *f, int fd, unsigned char *bu size_t size), FILE *f, int fd, size_t size) { - unsigned char *buf; - unsigned char *p; Dynstr *dynstr; + Dynstr *dynstr2; size_t n; - buf = alloc(size); - if (buf == NULL) + dynstr = new_dynstr(NULL, size, true); + if (dynstr == NULL) return NULL; - n = callback(f, fd, buf, size); + n = callback(f, fd, dynstr->mem, size); if (n == DYNSTR_ERROR) { - free(buf); + dynstr_free(dynstr); return NULL; } - + if (n < size) { - p = realloc(buf, n); - if (p != NULL) - buf = p; + dynstr2 = dynstr_substr(dynstr, 0, n); + if (dynstr2 == NULL) { + dynstr->size = n; + } else { + dynstr_free(dynstr); + dynstr = dynstr2; + } } - - dynstr = newstr(buf, n, false); - dynstr->dynamic = true; + return dynstr; } diff --git a/unittests.c b/unittests.c index 61a59bb..0ba5b33 100644 --- a/unittests.c +++ b/unittests.c @@ -253,19 +253,6 @@ static int test_new_returns_NULL_upon_first_allocation_failure(void) } -static int test_new_returns_NULL_upon_second_allocation_failure(void) -{ - Dynstr *dynstr; - - dynstr_init(); - dynstr_set_malloc(fail_malloc); - fail_malloc_after = 1; - dynstr = dynstr_new_empty(); - FAIL_UNLESS_EQUAL(dynstr, NULL); - return true; -} - - static int test_creates_from_cstring(void) { const char bytes[] = "asdfasdfafdasdfasdfqw4tb"; @@ -541,6 +528,7 @@ static int test_cats_ok(void) a = new_from_cstring("abc"); b = new_from_cstring("def"); c = dynstr_cat(a, b); + FAIL_UNLESS_EQUAL(dynstr_len(c), 6); size = dynstr_memcpy(buf, c, 0, dynstr_len(c)); FAIL_UNLESS_EQUAL(size, 6); FAIL_UNLESS_EQUAL(memcmp(buf, "abcdef", 6), 0); @@ -564,22 +552,6 @@ static int test_cat_returns_NULL_for_first_malloc_failure(void) } -static int test_cat_returns_NULL_for_second_malloc_failure(void) -{ - Dynstr *a; - Dynstr *b; - Dynstr *c; - - a = new_from_cstring("abc"); - b = new_from_cstring("def"); - dynstr_set_malloc(fail_malloc); - fail_malloc_after = 1; - c = dynstr_cat(a, b); - FAIL_UNLESS_EQUAL(c, NULL); - return true; -} - - static int test_byteat_reports_correct_character(void) { Dynstr *dynstr; @@ -1239,7 +1211,6 @@ static const struct test tests[] = { TEST(test_abort_handler_calls_abort), TEST(test_empty_string_is_empty), TEST(test_new_returns_NULL_upon_first_allocation_failure), - TEST(test_new_returns_NULL_upon_second_allocation_failure), TEST(test_creates_from_cstring), TEST(test_creates_from_memory), TEST(test_creates_from_constant_cstring), @@ -1258,7 +1229,6 @@ static const struct test tests[] = { TEST(test_copies_partial_substring_when_length_too_big), TEST(test_cats_ok), TEST(test_cat_returns_NULL_for_first_malloc_failure), - TEST(test_cat_returns_NULL_for_second_malloc_failure), TEST(test_byteat_reports_correct_character), TEST(test_byteat_reports_error_for_too_large_offset), TEST(test_byteat_reports_0xff_correctly), -- cgit v1.2.1