From 8cf5b6208cad807228a69d695e6521122d4b71da Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 17 Dec 2012 13:55:42 +1030 Subject: [PATCH] tal: don't automatically register cleanup function. It may interfere with other at_exit() calls, so let them call it manually. Also, use memset to zero, which really does make valgrind notice any leaks. Signed-off-by: Rusty Russell --- ccan/tal/tal.c | 18 ++++-------------- ccan/tal/tal.h | 11 +++++++++++ ccan/tal/test/run-allocfail.c | 1 + ccan/tal/test/run-array.c | 1 + ccan/tal/test/run-count.c | 2 ++ ccan/tal/test/run-destructor.c | 1 + ccan/tal/test/run-expand.c | 1 + ccan/tal/test/run-free.c | 1 + ccan/tal/test/run-groups-grow.c | 2 ++ ccan/tal/test/run-iter.c | 1 + ccan/tal/test/run-named-debug.c | 1 + ccan/tal/test/run-named-nolabels.c | 1 + ccan/tal/test/run-named.c | 1 + ccan/tal/test/run-notifier.c | 1 + ccan/tal/test/run-overflow.c | 1 + ccan/tal/test/run-steal.c | 1 + ccan/tal/test/run-take.c | 1 + ccan/tal/test/run-test-backend.c | 1 + ccan/tal/test/run.c | 1 + 19 files changed, 34 insertions(+), 14 deletions(-) diff --git a/ccan/tal/tal.c b/ccan/tal/tal.c index ce122070..1934a013 100644 --- a/ccan/tal/tal.c +++ b/ccan/tal/tal.c @@ -76,7 +76,6 @@ static void *(*allocfn)(size_t size) = malloc; static void *(*resizefn)(void *, size_t size) = realloc; static void (*freefn)(void *) = free; static void (*errorfn)(const char *msg) = (void *)abort; -static bool initialized = false; /* Count on non-destrutor notifiers; often stays zero. */ static size_t notifiers = 0; @@ -101,23 +100,19 @@ static struct children *ignore_destroying_bit(struct children *parent_child) } /* This means valgrind can see leaks. */ -static void tal_cleanup(void) +void tal_cleanup(void) { struct tal_hdr *i; - while ((i = list_top(&null_parent.c.children, struct tal_hdr, list))) + while ((i = list_top(&null_parent.c.children, struct tal_hdr, list))) { list_del(&i->list); + memset(i, 0, sizeof(*i)); + } /* Cleanup any taken pointers. */ take_cleanup(); } -/* For allocation failures inside ccan/take */ -static void take_alloc_failed(const void *p) -{ - tal_free(p); -} - /* We carefully start all real properties with a zero byte. */ static bool is_literal(const struct prop_hdr *prop) { @@ -344,11 +339,6 @@ static bool add_child(struct tal_hdr *parent, struct tal_hdr *child) struct children *children = find_property(parent, CHILDREN); if (!children) { - if (unlikely(!initialized)) { - atexit(tal_cleanup); - take_allocfail(take_alloc_failed); - initialized = true; - } children = add_child_property(parent, child); if (!children) return false; diff --git a/ccan/tal/tal.h b/ccan/tal/tal.h index 28be3f20..86b56d35 100644 --- a/ccan/tal/tal.h +++ b/ccan/tal/tal.h @@ -327,6 +327,17 @@ void tal_set_backend(void *(*alloc_fn)(size_t size), tal_expand_((void **)(a1p), (a2), sizeof**(a1p), \ (num2) + 0*sizeof(*(a1p) == (a2))) +/** + * tal_cleanup - remove pointers from NULL node + * + * Internally, tal keeps a list of nodes allocated from @ctx NULL; this + * prevents valgrind from noticing memory leaks. This re-initializes + * that list to empty. + * + * It also calls take_cleanup() for you. + */ +void tal_cleanup(void); + /** * tal_check - set the allocation or error functions to use diff --git a/ccan/tal/test/run-allocfail.c b/ccan/tal/test/run-allocfail.c index a0910cac..a166be3f 100644 --- a/ccan/tal/test/run-allocfail.c +++ b/ccan/tal/test/run-allocfail.c @@ -148,5 +148,6 @@ int main(void) ok1(err_count == when_to_fail - 1); tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-array.c b/ccan/tal/test/run-array.c index 6212dae5..d3001faf 100644 --- a/ccan/tal/test/run-array.c +++ b/ccan/tal/test/run-array.c @@ -42,5 +42,6 @@ int main(void) ok1(tal_first(parent) == NULL); tal_free(parent); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-count.c b/ccan/tal/test/run-count.c index 91b020dc..6a4eb4ab 100644 --- a/ccan/tal/test/run-count.c +++ b/ccan/tal/test/run-count.c @@ -82,5 +82,7 @@ int main(void) tal_free(p2); tal_free(p1); } + + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-destructor.c b/ccan/tal/test/run-destructor.c index 30c5136b..87354888 100644 --- a/ccan/tal/test/run-destructor.c +++ b/ccan/tal/test/run-destructor.c @@ -63,5 +63,6 @@ int main(void) tal_free(parent); ok1(destroy_count == 4); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-expand.c b/ccan/tal/test/run-expand.c index 2edb31f6..607947be 100644 --- a/ccan/tal/test/run-expand.c +++ b/ccan/tal/test/run-expand.c @@ -28,5 +28,6 @@ int main(void) tal_free(a); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-free.c b/ccan/tal/test/run-free.c index 7b9a086e..29aa8c6f 100644 --- a/ccan/tal/test/run-free.c +++ b/ccan/tal/test/run-free.c @@ -21,5 +21,6 @@ int main(void) tal_free(p); ok1(errno == EINVAL); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-groups-grow.c b/ccan/tal/test/run-groups-grow.c index e8e382db..ea379c08 100644 --- a/ccan/tal/test/run-groups-grow.c +++ b/ccan/tal/test/run-groups-grow.c @@ -42,5 +42,7 @@ int main(void) /* We can expect some residue from having any child, but limited! */ ok1(num_allocated <= allocated_after_first); + tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-iter.c b/ccan/tal/test/run-iter.c index 7021be27..561e09cc 100644 --- a/ccan/tal/test/run-iter.c +++ b/ccan/tal/test/run-iter.c @@ -29,5 +29,6 @@ int main(void) ok1(*p[i] == '1'); tal_free(p[i]); } + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-named-debug.c b/ccan/tal/test/run-named-debug.c index a6e51b43..679e6ec8 100644 --- a/ccan/tal/test/run-named-debug.c +++ b/ccan/tal/test/run-named-debug.c @@ -30,5 +30,6 @@ int main(void) ok1(strcmp(tal_name(p), __FILE__ ":29:int[]") == 0); tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-named-nolabels.c b/ccan/tal/test/run-named-nolabels.c index 512bee3f..fc7b81f5 100644 --- a/ccan/tal/test/run-named-nolabels.c +++ b/ccan/tal/test/run-named-nolabels.c @@ -26,5 +26,6 @@ int main(void) tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-named.c b/ccan/tal/test/run-named.c index acdc4513..d6275ac6 100644 --- a/ccan/tal/test/run-named.c +++ b/ccan/tal/test/run-named.c @@ -29,5 +29,6 @@ int main(void) ok1(strcmp(tal_name(p), "int[]") == 0); tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-notifier.c b/ccan/tal/test/run-notifier.c index f085b825..b57c902e 100644 --- a/ccan/tal/test/run-notifier.c +++ b/ccan/tal/test/run-notifier.c @@ -126,5 +126,6 @@ int main(void) tal_del_notifier(new_ctx, resize_notifier); tal_free(new_ctx); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-overflow.c b/ccan/tal/test/run-overflow.c index c5daf97d..473ba70a 100644 --- a/ccan/tal/test/run-overflow.c +++ b/ccan/tal/test/run-overflow.c @@ -94,5 +94,6 @@ int main(void) ok1(error_count == 3); tal_free(origpi); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-steal.c b/ccan/tal/test/run-steal.c index 3ff7b2b0..36251cb7 100644 --- a/ccan/tal/test/run-steal.c +++ b/ccan/tal/test/run-steal.c @@ -36,5 +36,6 @@ int main(void) ok1(tal_parent(p[4]) == p[0]); tal_free(p[0]); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-take.c b/ccan/tal/test/run-take.c index 72d2e8e3..94b65817 100644 --- a/ccan/tal/test/run-take.c +++ b/ccan/tal/test/run-take.c @@ -52,5 +52,6 @@ int main(void) ok1(tal_dup(NULL, char, take(c), 5, 5) == NULL); ok1(!taken_any()); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run-test-backend.c b/ccan/tal/test/run-test-backend.c index 66144cb3..8fdfc064 100644 --- a/ccan/tal/test/run-test-backend.c +++ b/ccan/tal/test/run-test-backend.c @@ -75,5 +75,6 @@ int main(void) /* Finally, free the parent. */ tal_free(p); + tal_cleanup(); return exit_status(); } diff --git a/ccan/tal/test/run.c b/ccan/tal/test/run.c index ab4341f2..98486512 100644 --- a/ccan/tal/test/run.c +++ b/ccan/tal/test/run.c @@ -56,5 +56,6 @@ int main(void) } tal_free(parent); + tal_cleanup(); return exit_status(); } -- 2.39.2