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 <rusty@rustcorp.com.au>

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();