From: Delio Brignoli Date: Sun, 12 Apr 2015 13:27:25 +0000 (+0200) Subject: tal_stack X-Git-Url: http://git.ozlabs.org/?p=ccan;a=commitdiff_plain;h=73930140fa25d3834570fcd906a01e199cfe17ac tal_stack Hi Rusty, Thanks for reviewing the patch. V2 is attached, see my comments below. > On 31 Mar 2015, at 02:36, Rusty Russell wrote: > > Delio Brignoli writes: >> Hi All, >> >> tal_stack implements a (trivial) stack of tal contexts. Would this be a worthy addition to CCAN? (not necessarily in its current form). […] > This is cute; I’ve seen similar used in Samba. It's Indeed, it was inspired by talloc_stack.h ;-) […] > You are missing a _info file: I would create that, and put your example > in an Example: section there. I moved the module and tests under can/tal/stack and added a LICENSE and _info. > Other random advice: > 1) You should also document the tal_newframe function (particularly note > that you're expected to tal_free the result, and that it will free > any future unfreed frames). And note that it’s not threadsafe. Done. > 2) You probably want tal_newframe to be a macro, and hand file and line > thought to the tal_alloc_ call. That makes debugging nicer when > you iterate the tree. Done. The macro is calling a tal_newframe_() function because I’d rather not make the module’s stack variable ‘public’. > 3) Consider whether you want to declare a dummy type 'struct tal_stack'. > Probably pretty unnecessary since it’s quite clear. Skipped this one. We can declare it later if we change our minds. Thanks — Delio From c2ceb9258d97b0dcb72e7b6986cfd2bd394b254e Mon Sep 17 00:00:00 2001 From: Delio Brignoli Date: Sun, 15 Mar 2015 13:26:40 +0100 Subject: [PATCH] tal_stack: new module - V2 Signed-off-by: Rusty Russell --- diff --git a/Makefile-ccan b/Makefile-ccan index cfaa0a9a..315962cf 100644 --- a/Makefile-ccan +++ b/Makefile-ccan @@ -100,6 +100,7 @@ MODS_WITH_SRC := antithread \ tal/grab_file \ tal/link \ tal/path \ + tal/stack \ tal/str \ tal/talloc \ talloc \ diff --git a/ccan/tal/stack/LICENSE b/ccan/tal/stack/LICENSE new file mode 120000 index 00000000..2b1feca5 --- /dev/null +++ b/ccan/tal/stack/LICENSE @@ -0,0 +1 @@ +../../../licenses/BSD-MIT \ No newline at end of file diff --git a/ccan/tal/stack/_info b/ccan/tal/stack/_info new file mode 100644 index 00000000..803a0058 --- /dev/null +++ b/ccan/tal/stack/_info @@ -0,0 +1,68 @@ +#include "config.h" +#include +#include + +/** + * tal/stack - stack of tal contextes (inspired by talloc_stack) + * + * Implement a stack of tal contexts. A new (empty) context is pushed on top + * of the stack using tal_newframe and it is popped/freed using tal_free(). + * tal_curframe() can be used to get the stack's top context. + * + * tal_stack can be used to implement per-function temporary allocation context + * to help mitigating memory leaks, but unlike the plain tal approach it does not + * require the caller to pass a destination context for returning allocated + * values. Instead, allocated values are moved to the parent context using + * tal_steal(tal_parent(tmp_ctx), ptr). + * + * Example: + * #include + * #include + * + * static int *do_work(void) + * { + * int *retval = NULL; + * tal_t *tmp_ctx = tal_newframe(); + * + * int *val = talz(tmp_ctx, int); + * assert(val != NULL); + * + * // ... do something with val ... + * + * if (retval >= 0) { + * // steal to parent cxt so it survives tal_free() + * tal_steal(tal_parent(tmp_ctx), val); + * retval = val; + * } + * tal_free(tmp_ctx); + * return retval; + * } + * + * int main(int argc, char *argv[]) + * { + * tal_t *tmp_ctx = tal_newframe(); + * int *val = do_work(); + * if (val) { + * // ... do something with val ... + * } + * // val is eventually freed + * tal_free(tmp_ctx); + * return 0; + * } + * + * License: BSD-MIT + * Author: Delio Brignoli + */ +int main(int argc, char *argv[]) +{ + /* Expect exactly one argument */ + if (argc != 2) + return 1; + + if (strcmp(argv[1], "depends") == 0) { + printf("ccan/tal\n"); + return 0; + } + + return 1; +} diff --git a/ccan/tal/stack/stack.c b/ccan/tal/stack/stack.c new file mode 100644 index 00000000..9b949e79 --- /dev/null +++ b/ccan/tal/stack/stack.c @@ -0,0 +1,24 @@ +/* Licensed under BSD-MIT - see LICENSE file for details */ + +#include +#include + +static tal_t *h = NULL; + +static void _free_frame(tal_t *o) +{ + h = tal_parent(o); +} + +tal_t *tal_newframe_(const char *label) +{ + h = tal_alloc_(h, 0, false, label); + assert(h != NULL); + tal_add_destructor(h, _free_frame); + return h; +} + +tal_t *tal_curframe(void) +{ + return h; +} diff --git a/ccan/tal/stack/stack.h b/ccan/tal/stack/stack.h new file mode 100644 index 00000000..24d909a8 --- /dev/null +++ b/ccan/tal/stack/stack.h @@ -0,0 +1,34 @@ +/* Licensed under BSD-MIT - see LICENSE file for details */ +#ifndef CCAN_TAL_STACK_H +#define CCAN_TAL_STACK_H + +#include + +/** + * tal_newframe - allocate and return a new nested tal context + * + * Allocates and push a new tal context on top of the stack. + * The context must be freed using tal_free() which will also pop it + * off the stack, which will also free all its nested contextes, if any. + * + * NOTE: this function is not threadsafe. + * + * Example: + * tal_t *ctx = tal_newframe(); + * // ... do something with ctx ... + * tal_free(ctx); + */ +#define tal_newframe(void) tal_newframe_(TAL_LABEL(tal_stack, "")); + +tal_t *tal_newframe_(const char *label); + +/** + * tal_curframe - return the current 'tal_stack frame' + * + * Returns the context currently on top of the stack. The initial context + * (before any tal_newframe() call) is the tal 'NULL' context. + * + * NOTE: this function is not threadsafe. + */ +tal_t *tal_curframe(void); +#endif /* CCAN_TAL_STACK_H */ diff --git a/ccan/tal/stack/test/run-stack.c b/ccan/tal/stack/test/run-stack.c new file mode 100644 index 00000000..0e5093c5 --- /dev/null +++ b/ccan/tal/stack/test/run-stack.c @@ -0,0 +1,35 @@ +#include +#include +#include + +int main(void) +{ + tal_t *parent, *cur; + + plan_tests(8); + + /* initial frame is NULL */ + ok1(tal_curframe() == NULL); + + /* create new frame and make sure all is OK */ + cur = tal_newframe(); + ok1(tal_curframe() == cur); + ok1(tal_parent(cur) == NULL); + + /* create another frame */ + parent = cur; + cur = tal_newframe(); + ok1(tal_curframe() == cur); + ok1(tal_parent(cur) == parent); + + /* unwind */ + tal_free(cur); + ok1(tal_curframe() == parent); + cur = tal_curframe(); + ok1(tal_parent(cur) == NULL); + tal_free(cur); + ok1(tal_curframe() == NULL); + + tal_cleanup(); + return exit_status(); +}