tal: handle take() pointers more carefully.

take() applies to the literal pointer value, so tal_free() means it may
randomly apply to a future allocation.  Similarly, tal_resize() should
preserve it.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c
index f6e2ee797c3e251dae5033bd928e7a61995b2fe6..dff0dcfba2058b0815f79423c0798af9b806fb67 100644 (file)
--- a/ccan/tal/tal.c
+++ b/ccan/tal/tal.c
@@ -385,6 +385,8 @@ static void del_tree(struct tal_hdr *t, const tal_t *orig, int saved_errno)
 {
        struct prop_hdr **prop, *p, *next;
 
+       assert(!taken(from_tal_hdr(t)));
+
         /* Already being destroyed?  Don't loop. */
         if (unlikely(get_destroying_bit(t->parent_child)))
                 return;
@@ -710,6 +712,10 @@ bool tal_resize_(tal_t **ctxp, size_t size, size_t count, bool clear)
                /* Fix up linked list pointers. */
                t->list.next->prev = t->list.prev->next = &t->list;
 
+               /* Copy take() property. */
+               if (taken(from_tal_hdr(old_t)))
+                       take(from_tal_hdr(t));
+
                /* Fix up child property's parent pointer. */
                child = find_property(t, CHILDREN);
                if (child) {

diff --git a/ccan/tal/tal.h b/ccan/tal/tal.h
index 865a696713c88d9243f64515200b49c7c164e554..e25dcb92985922acc5a50007f5c397ddb4aa9237 100644 (file)
--- a/ccan/tal/tal.h
+++ b/ccan/tal/tal.h
@@ -106,6 +106,9 @@ void *tal_free(const tal_t *p);
  * This returns true on success (and may move *@p), or false on failure.
  * On success, tal_count() of *@p will be @count.
  *
+ * Note: if *p is take(), it will still be take() upon return, even if it
+ * has been moved.
+ *
  * Example:
  *     tal_resize(&p, 100);
  */

diff --git a/ccan/tal/test/run-take-free-assert.c b/ccan/tal/test/run-take-free-assert.c
new file mode 100644 (file)
index 0000000..062a778
--- /dev/null
+++ b/ccan/tal/test/run-take-free-assert.c
@@ -0,0 +1,44 @@
+#include <ccan/tal/tal.h>
+#include <ccan/tal/tal.c>
+#include <ccan/tap/tap.h>
+#include <signal.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+int main(void)
+{
+       int status;
+       char *p;
+
+       plan_tests(4);
+
+       /* Test direct free. */
+       switch (fork()) {
+       case 0:
+               tal_free(take(tal(NULL, char)));
+               exit(2);
+       case -1:
+               exit(1);
+       default:
+               wait(&status);
+               ok1(WIFSIGNALED(status));
+               ok1(WTERMSIG(status) == SIGABRT);
+       }
+
+       /* Test indirect free. */
+       switch (fork()) {
+       case 0:
+               p = tal(NULL, char);
+               take(tal(p, char));
+               tal_free(p);
+               exit(2);
+       case -1:
+               exit(1);
+       default:
+               wait(&status);
+               ok1(WIFSIGNALED(status));
+               ok1(WTERMSIG(status) == SIGABRT);
+       }
+       return exit_status();
+}

diff --git a/ccan/tal/test/run-take.c b/ccan/tal/test/run-take.c
index d93304e206bdf4207cb33bc7bd5eaf12333eb982..4a7f0c7495374fa72696dce45e6111ca024fec09 100644 (file)
--- a/ccan/tal/test/run-take.c
+++ b/ccan/tal/test/run-take.c
@@ -6,7 +6,7 @@ int main(void)
 {
        char *parent, *c;
 
-       plan_tests(21);
+       plan_tests(22);
 
        /* We can take NULL. */
        ok1(take(NULL) == NULL);
@@ -44,6 +44,12 @@ int main(void)
        tal_free(c);
        ok1(tal_first(parent) == NULL);
 
+       /* tal_resize should return a taken pointer. */
+       c = take(tal_arr(parent, char, 5));
+       tal_resize(&c, 100);
+       ok1(taken(c));
+       tal_free(c);
+
        tal_free(parent);
        ok1(!taken_any());