From 7ca46909f86e8bbad4e4bb19cac1a75d4ebf3df6 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 20 Nov 2012 15:47:30 +1030 Subject: [PATCH] tal: Make tal_resize() easier to use. Instead of trying to force people to use the return value, pass a pointer. This makes it easier if you want to handle failure: no overwriting the old pointer! Signed-off-by: Rusty Russell --- ccan/tal/tal.c | 20 +++++++++++-------- ccan/tal/tal.h | 36 ++++++++++++++++++++++++++++------- ccan/tal/test/run-allocfail.c | 32 ++++++++++++++++++++++++++++--- ccan/tal/test/run-array.c | 10 +++++++++- 4 files changed, 79 insertions(+), 19 deletions(-) diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c index f33a069a..ffe2d713 100644 --- a/ccan/tal/tal.c +++ b/ccan/tal/tal.c @@ -657,22 +657,23 @@ tal_t *tal_parent(const tal_t *ctx) return from_tal_hdr(group->parent_child->parent); } -void *tal_realloc_(tal_t *ctx, size_t size) +bool tal_resize_(tal_t **ctxp, size_t size) { struct tal_hdr *old_t, *t, **prev; struct group *group; struct children *child; - old_t = debug_tal(to_tal_hdr(ctx)); + old_t = debug_tal(to_tal_hdr(*ctxp)); t = resizefn(old_t, size + sizeof(struct tal_hdr)); if (!t) { call_error("Reallocation failure"); - tal_free(old_t); - return NULL; + return false; } + + /* If it didn't move, we're done! */ if (t == old_t) - return ctx; + return true; update_bounds(t); /* Fix up linked list pointer. */ @@ -692,8 +693,8 @@ void *tal_realloc_(tal_t *ctx, size_t size) assert(child->parent == old_t); child->parent = t; } - - return from_tal_hdr(debug_tal(t)); + *ctxp = from_tal_hdr(debug_tal(t)); + return true; } char *tal_strdup(const tal_t *ctx, const char *p) @@ -758,7 +759,10 @@ char *tal_vasprintf(const tal_t *ctx, const char *fmt, va_list ap) if (ret < max) break; - buf = tal_resize(buf, max *= 2); + if (!tal_resize(&buf, max *= 2)) { + tal_free(buf); + buf = NULL; + } } if (ctx == TAL_TAKE) tal_free(fmt); diff --git a/ccan/tal/tal.h b/ccan/tal/tal.h index a12e0dc4..28726c32 100644 --- a/ccan/tal/tal.h +++ b/ccan/tal/tal.h @@ -38,6 +38,10 @@ typedef void tal_t; * Allocates a specific type, with a given parent context. The name * of the object is a string of the type, but if CCAN_TAL_DEBUG is * defined it also contains the file and line which allocated it. + * + * Example: + * int *p = tal(NULL, int); + * *p = 1; */ #define tal(ctx, type) \ ((type *)tal_alloc_((ctx), sizeof(type), false, TAL_LABEL(type, ""))) @@ -48,6 +52,10 @@ typedef void tal_t; * @type: the type to allocate. * * Equivalent to tal() followed by memset() to zero. + * + * Example: + * p = talz(NULL, int); + * assert(*p == 0); */ #define talz(ctx, type) \ ((type *)tal_alloc_((ctx), sizeof(type), true, TAL_LABEL(type, ""))) @@ -60,6 +68,9 @@ typedef void tal_t; * children (recursively) before finally freeing the memory. * * Note: errno is preserved by this call. + * + * Example: + * tal_free(p); */ void tal_free(const tal_t *p); @@ -68,6 +79,11 @@ void tal_free(const tal_t *p); * @ctx: NULL, or tal allocated object to be parent. * @type: the type to allocate. * @count: the number to allocate. + * + * Example: + * p = tal_arr(NULL, int, 2); + * p[0] = 0; + * p[1] = 1; */ #define tal_arr(ctx, type, count) \ ((type *)tal_alloc_((ctx), tal_sizeof_(sizeof(type), (count)), false, \ @@ -78,21 +94,27 @@ void tal_free(const tal_t *p); * @ctx: NULL, or tal allocated object to be parent. * @type: the type to allocate. * @count: the number to allocate. + * + * Example: + * p = tal_arrz(NULL, int, 2); + * assert(p[0] == 0 && p[1] == 0); */ #define tal_arrz(ctx, type, count) \ ((type *)tal_alloc_((ctx), tal_sizeof_(sizeof(type), (count)), true, \ TAL_LABEL(type, "[]"))) /** - * tal_resize - enlarge or reduce a tal_arr(z). - * @p: The tal allocated array to resize. + * tal_resize - enlarge or reduce a tal_arr[z]. + * @p: A pointer to the tal allocated array to resize. * @count: the number to allocate. * - * This returns the new pointer, or NULL (and destroys the old one) - * on failure. + * This returns true on success (and may move *@p), or false on failure. + * + * Example: + * tal_resize(&p, 100); */ #define tal_resize(p, count) \ - ((tal_typeof(p) tal_realloc_((p), tal_sizeof_(sizeof(*p), (count))))) + tal_resize_((void **)(p), tal_sizeof_(sizeof**(p), (count))) /** * tal_steal - change the parent of a tal-allocated pointer. @@ -102,7 +124,7 @@ void tal_free(const tal_t *p); * This may need to perform an allocation, in which case it may fail; thus * it can return NULL, otherwise returns @ptr. * - * Weird macro avoids gcc's 'warning: value computed is not used'. + * Note: weird macro avoids gcc's 'warning: value computed is not used'. */ #if HAVE_STATEMENT_EXPR #define tal_steal(ctx, ptr) \ @@ -301,7 +323,7 @@ void *tal_alloc_(const tal_t *ctx, size_t bytes, bool clear, const char *label); tal_t *tal_steal_(const tal_t *new_parent, const tal_t *t); -void *tal_realloc_(tal_t *ctx, size_t size); +bool tal_resize_(tal_t **ctxp, size_t size); bool tal_add_destructor_(tal_t *ctx, void (*destroy)(void *me)); diff --git a/ccan/tal/test/run-allocfail.c b/ccan/tal/test/run-allocfail.c index dd6abff4..a0910cac 100644 --- a/ccan/tal/test/run-allocfail.c +++ b/ccan/tal/test/run-allocfail.c @@ -15,6 +15,15 @@ static void *failing_alloc(size_t len) return malloc(len); } +static void *failing_realloc(void *p, size_t len) +{ + if (alloc_count++ == when_to_fail) + return NULL; + + return realloc(p, len); +} + + static void nofail_on_error(const char *msg) { diag("ERROR: %s", msg); @@ -27,12 +36,12 @@ static void destroy_p(void *p) int main(void) { - void *p, *c1, *c2; + char *p, *c1, *c2; bool success; - plan_tests(21); + plan_tests(25); - tal_set_backend(failing_alloc, NULL, NULL, nofail_on_error); + tal_set_backend(failing_alloc, failing_realloc, NULL, nofail_on_error); /* Fail at each possible point in an allocation. */ when_to_fail = err_count = 0; @@ -56,6 +65,23 @@ int main(void) ok1(when_to_fail > 1); ok1(err_count == when_to_fail - 1); + /* Now during resize. */ + c2 = c1; + when_to_fail = err_count = 0; + for (;;) { + alloc_count = 0; + if (tal_resize(&c1, 100)) + break; + /* Failing alloc will not change pointer. */ + ok1(c1 == c2); + when_to_fail++; + }; + ok1(alloc_count == 1); + ok1(when_to_fail == 1); + ok1(err_count == 1); + /* Make sure it's really resized. */ + memset(c1, 1, 100); + /* Now for second child. */ when_to_fail = err_count = 0; do { diff --git a/ccan/tal/test/run-array.c b/ccan/tal/test/run-array.c index 53392da3..a87650c3 100644 --- a/ccan/tal/test/run-array.c +++ b/ccan/tal/test/run-array.c @@ -7,7 +7,7 @@ int main(void) char *parent, *c[4]; int i; - plan_tests(9); + plan_tests(11); parent = tal(NULL, char); ok1(parent); @@ -33,6 +33,14 @@ int main(void) strcpy(c[i], "abc"); tal_free(c[i]); } + + /* Resizing. */ + c[0] = tal_arrz(parent, char, 4); + ok1(tal_resize(&c[0], 6)); + strcpy(c[0], "hello"); + tal_free(c[0]); + ok1(tal_first(parent) == NULL); + tal_free(parent); return exit_status(); -- 2.39.2