tal: Make tal_resize() easier to use.
authorRusty Russell <rusty@rustcorp.com.au>
Tue, 20 Nov 2012 05:17:30 +0000 (15:47 +1030)
committerRusty Russell <rusty@rustcorp.com.au>
Tue, 20 Nov 2012 05:17:30 +0000 (15:47 +1030)
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 <rusty@rustcorp.com.au>
ccan/tal/tal.c
ccan/tal/tal.h
ccan/tal/test/run-allocfail.c
ccan/tal/test/run-array.c

index f33a069a4bf49af63932350aa236b513cfc912e4..ffe2d71334dce4473d57243bca0827aaa5fee565 100644 (file)
@@ -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);
index a12e0dc476ba6acb773e2e65d0b2de68a1c8f234..28726c324cedd9ec4c150bcd02f369c6e6180905 100644 (file)
@@ -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));
 
index dd6abff4dc6f7a0678bb3055305330382516143e..a0910cac5441dcf4fa5097798610134aa34b07c5 100644 (file)
@@ -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 {
index 53392da3a902f35689120908959b121786763c69..a87650c346a06eccdbfa739b39536cb2502e7065 100644 (file)
@@ -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();